We built an(other) object factory module for our current project and it looks a lot like all the others:
The Big Design Refactor
What is The Big Design Refactor?
Design starts with systematic thinking and then shifts to incremental changes. No matter where a project starts, at some point the design systems’ integrity will degrade to the point that you need to look at the whole thing fresh. Welcome to the Big Design Refactor. In the beginning, you had a beautiful, functional design system. And then you had to watch, helplessly, as it degraded under the weight of a thousand tiny changes. It’s maddening, and a big reason many designers are allergic to Agile methodologies.
What can you do? Understanding and accepting that this design-system degradation is an affordance of differently-sized design and development cycles is the first step towards making peace. Once you realize and accept this change, it’s much easier to deal with. Plan on it. Talk about it with your team. Designers need not work at developers tempo. Rather, they should strive to stay in-rhythm with development, working at their own pace, and making sure their beats and decision-points intersect with development at regular intervals.
As the designer, it’s your job to keep an eye on the health of the graphic system. Just as the developers incur and pay down their technical debt, you’ll manage your design debt. How? Mention that the graphic system is starting to break down. All the while, you’re still doing your work. Keep solving the tactical problems, keep delivering value. At some point, the balance shifts: you’re no longer plugging little leaks or engaging in preventative maintenance. The system is undermined; the cracks are starting to show. Now’s the time to have a talk with your team. You’re going to need a few weeks to work on this. You’re going to need the development team to find something design-light and backend-heavy to focus on for a week or two. Hold a design retro. Get their feedback, and their buy-in, and their good ideas. And now you’ll have a break from the tactical work of patching up design as features iterate. You’re a Pure Designer again, working in your idiom, experimenting and sketching and building a new design system.
How does the Big Design Refactor work?
When I’m in the midst of a Big Design Refactor, I’m spending most of my day in Adobe Creative Suite. I’m pulling the product team over ~4-7 times a week to bounce ideas off of them. I’m pinging the development team ~1-2 times a week to consult on the technical implications of where I’m going. By the end of the week or two, I’m usually delivering a set of user stories accompanied by mockups. It’ll often take an IPM or two to get through all of them, and it’s important that they get implemented soon. Nothing feels more like waste than a heavy investment in design, followed by unacted-upon stories that go stale. This will kill trust between the design and product teams (in both directions); it’s downright poisonous.
Now, while the new design is being built by developers, I’ll occasionally hop back into Adobe Creative Suite for assets or newly-discovered UX tweaks, but most of my time is spent pairing with developers. I’ll also keep a close eye on acceptance. Pixel-perfection is rarely necessary, but miss an important UX point now and the error will become enshrined as part of the system.
Once the Design Refactor has been assimilated into the app—and it’s rare that 100% goes in, but 80-90% is the norm—it’s Tactical Incremental Fun Time! I’ll spend most of this time pairing on stories, picking of styling stories to solo on, and working on design problems revealed by user testing. At this point it’s probably 66% development and 33% Adobe apps. The debt clock is starting to tick again, and once the pain is noticeable, I’ll start making noises: “we’re ok for right this second, but we’re going to need a design refactor in the next 3-5 weeks”.
Why are the Rhythms Different?
Design and development are activities that move at different speeds. Test-driven development and Agile project management allow developers to break work down into small stories and iterate on them. The unit of work for the “what” of the story is “what’s the smallest possible thing that delivers value to the user?” and the “how” of the story is “what’s the simplest possible thing that can work?”. These units of work tend to translate poorly to design, because effective graphic design is almost always a system. Changing arbitrary pieces tends to degrade the whole. Design adjustments that are close in size to development units-of-work can be made, but they inevitably undermine the graphic system, creating Design Debt. Debt is fine, if used responsibly, but it needs to be paid down at some point. (More on that another time.)
Developers work at a quick tempo. They use Agile’s small units of work to facilitate a supple workflow that responds gracefully to changing client needs.
The customer wants to re-prioritize a feature? No problem! Move it to the top of the backlog and we’ll get to it next.
Because they’re backed by tests, devs are free to move around the codebase and project, making changes where the customer likes, with the confidence that their test suite will protect them from breaking the app. Designers have no such safety net—which is one reason that developing meaningful automated testing for design is crucial to a mature Agile design practice.
The rhythm of design is slower. Design pulls together an information architecture, UX metaphors, visual styles, a typographic system, a color system. Visual rhythm, hierarchies, and scale combine into a whole graphic system—more than the sum of its parts. All these pieces interrelate, and changes cascade into other. While adjustments to individual design elements can happen quickly, feature-scale iteration doesn’t allow for changes to the system; those take more time.
TL;DR
Ideally, bring a cohesive graphic system to Inception. Accept that it will degrade over time. Make tactical adjustments to keep pace with agile development, and plan on overhauling the design system every quarter or so.
Refactoring the Deeply-Nested Hash Antipattern
On a few different Ruby projects, I have seen an antipattern emerge involving nested hashes.
For example, consider this simple question-and-answer command-line script:
QUIZ = {
question_1: {
prompt: "What is your name?"
},
question_2: {
prompt: "What is your favorite color?"
}
}
answers = {}
QUIZ.each do |name, question_hash|
print question_hash[:prompt], " "
answers[name] = gets.chomp
end
p answers
The QUIZ constant isn’t too hard to understand. The keys (question_1 and question_2) each point to a value that is itself another hash. Let’s call that a question_hash. For now a question_hash has only one key-value pair, the prompt.
The answers hash simply accumulates answers. The key is the key from the QUIZ constant, and the value is the answer the user typed in.
At the end of the script, the program calls Kernel#p to print out the inspect string for the answers hash.
Sure enough, the program works.
$ ruby quiz.rb
What is your name? Grant
What is your favorite color? orange
{:question_1=>"Grant", :question_2=>"orange"}Take a look at the QUIZ.each block up there. Notice that the inside of the block assumes that the question_hash will have a :prompt key.
That’s not too complicated. But let’s add some more features to our script. Now we will add a multiple-choice question to the QUIZ hash:
QUIZ = {
question_1: {
prompt: "What is your name?"
},
question_2: {
prompt: "What is your favorite color?"
},
question_3: {
prompt: "What is your favorite integer between 1 and 5?",
choices: %w[1 2 3 4 5]
}
}
Now we want to treat a question_hash differently if it has a choices key. One way to do this is to write two different methods that take the question_hash as an argument and ask the question. The methods will return the answer that the user inputs.
def simple_answer(question_hash)
print question_hash[:prompt], " "
gets.chomp
end
def multiple_choice_answer(question_hash)
choices = question_hash[:choices]
answer = nil
until choices.include?(answer)
print question_hash[:prompt], " "
answer = gets.chomp
puts "You must pick an answer from #{choices}" unless choices.include?(answer)
end
answer
end
Now we simply update our block to call the appropriate method for each question_hash.
QUIZ.each do |name, question_hash|
answer = if question_hash[:choices]
multiple_choice_answer(question_hash)
else
simple_answer(question_hash)
end
answers[name] = answer
end
p answers
This all still works fine.
$ ruby quiz.rb
What is your name? Grant
What is your favorite color? orange
What is your favorite integer between 1 and 5? 7
You must pick an answer from ["1", "2", "3", "4", "5"]
What is your favorite integer between 1 and 5? 3
{:question_1=>"Grant", :question_2=>"orange", :question_3=>"3"}But the code is very quickly getting more complex. Before long, we will have questions with sub-questions, conditional follow-up questions, and even more complicated features. Here’s an example that supports a Proc for formatting the user’s answer.
QUIZ = {
question_1: {
prompt: "What is your name?"
},
question_2: {
prompt: "What is your favorite color?",
display_proc: ->(color) { "The color is #{color}." }
},
question_3: {
prompt: "What is your favorite integer between 1 and 5?",
choices: %w[1 2 3 4 5]
}
}
answers = {}
def simple_answer(question_hash)
print question_hash[:prompt], " "
gets.chomp
end
def multiple_choice_answer(question_hash)
choices = question_hash[:choices]
answer = nil
until choices.include?(answer)
print question_hash[:prompt], " "
answer = gets.chomp
puts "You must pick an answer from #{choices}" unless choices.include?(answer)
end
answer
end
QUIZ.each do |name, question_hash|
answer = if question_hash[:choices]
multiple_choice_answer(question_hash)
else
simple_answer(question_hash)
end
answers[name] = answer
end
display_answers = Hash[
answers.map do |name, answer|
if display_proc = QUIZ[name][:display_proc]
[name, display_proc[answer]]
else
[name, answer]
end
end
]
p display_answers
The output now looks like this
$ ruby quiz.rb
What is your name? Grant
What is your favorite color? orange
What is your favorite integer between 1 and 5? 6
You must pick an answer from ["1", "2", "3", "4", "5"]
What is your favorite integer between 1 and 5? 3
{:question_1=>"Grant", :question_2=>"The color is orange.", :question_3=>"3"}And the code will keep growing and growing, making it harder for the next developer to understand exactly how the big Hash will translate into the user’s experience.
Take a look at those answer methods. Whenever I see a bunch of methods that accept the same argument, I suspect that there is an object missing in my system. Let’s create a new Question object. Imagine that we have it in that QUIZ.each block, and that we can simply ask the Question for its answer directly.
class Question
def initialize(question_hash)
@question_hash = question_hash
end
def answer
if @question_hash[:choices]
multiple_choice_answer(@question_hash)
else
simple_answer(@question_hash)
end
end
private
def simple_answer(question_hash)
print question_hash[:prompt], " "
gets.chomp
end
def multiple_choice_answer(question_hash)
choices = question_hash[:choices]
answer = nil
until choices.include?(answer)
print question_hash[:prompt], " "
answer = gets.chomp
puts "You must pick an answer from #{choices}" unless choices.include?(answer)
end
answer
end
end
QUIZ.each do |name, question_hash|
question = Question.new(question_hash)
answers[name] = question.answer
end
Notice that simple_answer and multiple_choice_answer are now completely private. That’s good, because it hides away the internal details of how a Question works. We are now free to start doing some refactoring, without worrying about the rest of the code in the system breaking.
Our first refactor will be to remove the arguments from the private methods, since it’s now possible to get at the @question_hash directly from within the Question object.
class Question
def initialize(question_hash)
@question_hash = question_hash
end
def answer
if @question_hash[:choices]
multiple_choice_answer
else
simple_answer
end
end
private
def simple_answer
print @question_hash[:prompt], " "
gets.chomp
end
def multiple_choice_answer
choices = @question_hash[:choices]
answer = nil
until choices.include?(answer)
print @question_hash[:prompt], " "
answer = gets.chomp
puts "You must pick an answer from #{choices}" unless choices.include?(answer)
end
answer
end
end
Next, notice that the only reason we are passing the question_hash into the Question is to get out the prompt and the choices. When a constructor takes a hash with expected keys, you can swap the hash out for multiple arguments instead.
class Question
def initialize(prompt, choices)
@prompt = prompt
@choices = choices
end
def answer
if @choices
multiple_choice_answer
else
simple_answer
end
end
private
def simple_answer
print @prompt, " "
gets.chomp
end
def multiple_choice_answer
answer = nil
until @choices.include?(answer)
print @prompt, " "
answer = gets.chomp
puts "You must pick an answer from #{@choices}" unless @choices.include?(answer)
end
answer
end
end
Now the object is easier to understand. But there’s still a problem. To use this new object, the each block needs to be updated:
QUIZ.each do |name, question_hash|
prompt = question_hash[:prompt]
choices = question_hash[:choices]
question = Question.new(prompt, choices)
answers[name] = question.answer
end
This is uglier than what we had before. But there’s a quick fix that makes our code much nicer. Let’s change the QUIZ hash so that its values are already Question objects:
QUIZ = {
question_1: Question.new("What is your name?", nil),
question_2: Question.new("What is your favorite color?", nil),
question_3: Question.new("What is your favorite integer between 1 and 5?",
%w[1 2 3 4 5])
}
This makes our each block much nicer!
QUIZ.each do |name, question|
answers[name] = question.answer
end
But there’s still one problem. Our QUIZ no longer holds the display_proc, so this code from before fails:
display_answers = Hash[
answers.map do |name, answer|
if display_proc = QUIZ[name][:display_proc]
[name, display_proc[answer]]
else
[name, answer]
end
end
]
So let’s put that proc into the Question object and make it available via an attr_reader. While we’re at it, we can give default values of nil for both choices and nil.
class Question
attr_reader :display_proc
def initialize(prompt, choices = nil, display_proc = nil)
@prompt = prompt
@choices = choices
@display_proc = display_proc
end
# ...
Now our QUIZ looks like this:
QUIZ = {
question_1: Question.new("What is your name?"),
question_2: Question.new("What is your favorite color?",
nil,
->(color) { "The color is #{color}." }),
question_3: Question.new("What is your favorite integer between 1 and 5?",
%w[1 2 3 4 5])
}
And our display_answers code looks like this:
display_answers = Hash[
answers.map do |name, answer|
if display_proc = QUIZ[name].display_proc
[name, display_proc[answer]]
else
[name, answer]
end
end
]
Now let’s move the logic for deciding how an answer should be formatted into the Question object. (We can remove the attr_reader for display_proc at this time as well, to keep things nicely hidden away.)
class Question
# ...
def display_answer(answer)
if @display_proc
@display_proc[answer]
else
answer
end
end
# ...
Which gives us:
display_answers = Hash[
answers.map do |name, answer|
question = QUIZ[name]
[name, question.display_answer(answer)]
end
]
At this point, it seems like the question name could also easily be put into the Question object as a new first argument. Let’s replace the QUIZ constant with a QUESTIONS constant that holds an array of Question objects.
QUESTIONS = [
Question.new(:question_1,
"What is your name?"),
Question.new(:question_2,
"What is your favorite color?",
nil,
->(color) { "The color is #{color}." }),
Question.new(:question_3,
"What is your favorite integer between 1 and 5?",
%w[1 2 3 4 5])
]
answers = {}
QUESTIONS.each do |question|
answers[question.name] = question.answer
end
display_answers = Hash[
QUESTIONS.map do |question|
answer = answers[question.name]
[question.name, question.display_answer(answer)]
end
]
p display_answers
Now the code is much cleaner. There are definitely more places the code could be refactored. choices and display_proc could be moved into an options hash argument since they are truly optional. Maybe there could be a SimpleQuestion superclass and a subclass called MultipleChoiceQuestion with all the choices-related code.
The most important thing to realize is that now the conversation that future developers might have about this code has shifted.
Before, the conversation would probably have centered around how complicated the code looked and confusion about how the structure of the QUIZ hash affects the control flow of the rest of the program. What is the easiest way to make a small change to add a new feature without breaking the whole big complex structure?
Now, the conversation can be about whether the current object model makes sense or should be tweaked in some small way. And with the full power of a Ruby class instead of the limited behavior of a Hash, the developers will have more options.
This code example is available on GitHub with a full Git history of the working code at each stage. Here’s the final code:
class Question
attr_reader :name
def initialize(name, prompt, choices = nil, display_proc = nil)
@name = name
@prompt = prompt
@choices = choices
@display_proc = display_proc
end
def answer
if @choices
multiple_choice_answer
else
simple_answer
end
end
def display_answer(answer)
if @display_proc
@display_proc[answer]
else
answer
end
end
private
def simple_answer
print @prompt, " "
gets.chomp
end
def multiple_choice_answer
answer = nil
until @choices.include?(answer)
print @prompt, " "
answer = gets.chomp
puts "You must pick an answer from #{@choices}" unless @choices.include?(answer)
end
answer
end
end
QUESTIONS = [
Question.new(:question_1,
"What is your name?"),
Question.new(:question_2,
"What is your favorite color?",
nil,
->(color) { "The color is #{color}." }),
Question.new(:question_3,
"What is your favorite integer between 1 and 5?",
%w[1 2 3 4 5])
]
answers = {}
QUESTIONS.each do |question|
answers[question.name] = question.answer
end
display_answers = Hash[
QUESTIONS.map do |question|
answer = answers[question.name]
[question.name, question.display_answer(answer)]
end
]
p display_answers
Migrating from a single Rails app to a suite of Rails engines
TL;DR
We moved a Rails app into an unbuilt engine of a new blank slate container app to allow new parts of our app to live next to it as engines as well. It has been working great for us!
I have a sample app rails_container_and_engines of the result’s structure on github.
Skip to the pitfalls and discoveries section to read about some of the speed bumps we during our transition. Interested in the why and how? Read on!
Rationale
As part of the project we had built a web app, a mobile app that talks to the web app’s API, two ETL tools to 1) do the initial data import from the app we were replacing and 2) get data into another of our client’s apps. At this point we knew that we would create one more big web app and several auxiliary apps.
Running up to the decision to using unbuilt engines we had several intense discussions on how to build loosely-coupled, highly-cohesive systems with Rails applications. We saw basically three choices:
- All-in-one app that could get more structure through namespacing of its interals.
- Services (REST or whatever, but) running as separate apps and communicating via APIs.
- Engines running within a mostly feature-less container app.
The two big web apps share several models within the data access layer and use the same data. Because of this we chose the third option and left all of the apps within the same rails code base.
Internal discussions highlighted the costs/benefits of having all of these apps live within their own Rails project versus an engines approach. We feel that by using engines we are getting many of the benefits of a component based architecture without breaking Rails patterns. In addition, we feel that the cost of maintaining individual applications that share a central database or one giant application with less defined components would have been very high.
In order to make day to day development easier, and to avoid the “where do migrations live…” conversation and top level Rails deployment patterns, there is one twist to the architecture: everything resides in one git repo and engines are referenced from a single container application (similar to old school enterprise archive files). Each application is exposed via a unique context or resource identifier (each engine/app could also be isolated per instance via Apache). Here is the directory structure we ended up with:
container_rails_app/
...
app
config
engines/
etl/
shared_modules/
web_app_1/
web_app_2/
...
Mike described this pattern in his recent blog posts Unbuilt Rails Dependencies: How to design for loosely-coupled, highly-cohesive components within a Rails application and Rails Contained: A Container for Web Application Development and Deployment. On how to make RubyMine work seamlessly with engines, read my post on IntelliJ Modules in Rubymine.
The steps we took
- Generate a new, empty Rails app
- Within the
enginesfolder, create a new, mountable Rails engine Copy all the tests from the original Rails app into the test directory and make them green.
Ok. This step is a bit more involved. Essentially that’s it thought. Find some of the pitfalls we ran into described below. Here are a couple of highlights of what needs to happen:
- Copy the files you need for a test to pass and namespace its class
- Start with the model tests and work your way up towards integration and acceptance tests
- Namespace everything with the name of the engine (either by fully qualifying every name – don’t do that) or with the lexical scope trick explained below. This includes tests and all classes.
- Load needed gems explicitly: engines don’t load as much automatically as a Rails app
- Namespace tables and assets
- Copy the old .git directory into the new root folder in order to not loose any history. For us, git was not able to detect the changes as moves in many cases. This is certainly an area where you can improve on our solution!
For us, the real work started after this, when we started pullling out common code into a common models engine and began work on the second app.
We got all the tests to pass before we had namespaced any of the assets or rake tasks. That was an additional search-and-replace heavy step after the actual transition to an engine. It is not necessary right away if you do not have multiple applications at first, but to achieve the full effect, you will need to namespace the things as well.
Pitfalls and discoveries
Namespaces
Modules in your enige are not automatically loaded: make sure you reference them yourself before they are needed in other files.
Asset pre-compilation
Rails creates a default app/assets/stylesheets/application.css file which contains these lines:
/* ...
*= require_self
*= require_tree .
*/
If you have this file in your main app, all css files will be compiled into one file. For us, this made almost everything look right. Almost. Little things broke here and there. Our app contained a couple of sections for which the stylesheets were meant to be loaded separately. Add files that you want to have precompiled as individual files to config.assets.precompile list to have them precompiled into separate files and solve this problem.
Are all your references and associations breaking?
Try this
class M::Y
def use_y
M::Y.do_it!
end
end
class M::Y
def self.do_it!
end
end
instead of
module M
class X
def use_y
Y.do_it!
end
end
class Y
def self.do_it!
end
end
end
The first way sets the lexical scope to the module M as well as to the class Y allows you to references other classes in M without their full name. The second way sets the scope only to class M::Y and you have to fully qualify every class name to find it.
Don’t fight conventions
We were using fixture builder and while we were namespacing classes in modules we were trying to override the default table names to not be namespaced… Fixture builder didn’t like that at all. There may or may not be other dependencies that make leaving the conventions hard. So, save yourself the trouble and do the migrations (and stay consistent!) and namespace your tables as well!
HABTM
They seem to be falling out of favor, which is probably a good thing. With engines they don’t seem to work (well). We ended up getting rid of our last two habtm relationships instead of trying to make them work. Creating the join as a class and adding the necessary has many :through relationships is straight forward enough.
We ended up with one production performance bug due to this: Rails
Engines depending on engines
We used kaminari for our pagination requirements. When our app first became an engine, kaminari stopped picking up our custom views. Instead it used its standard views. The load order got screwed up to which there are basically two solutions:
- require => nil on both engines in the Gemfile and force the correct load order in your app, or…
- avoid name clashes by namespacing your views (which you were going to do anyways, right?)
Simple DRY Validations
Here’s a handy trick for making custom validations easily reusable.
This is an extract from a customer model with three different street addresses, in which we validate all three of the zip codes. (In this code, the GeoState.valid_zip_code? method answers if something that looks like a zip code is an actual zip code – not all five digit combinations are in use as zip codes, and we want to make sure we’ve got a live one.)
def validate_home_zip_code
validate_zip_code(:home_zip_code)
end
def validate_mailing_zip_code
validate_zip_code(:mailing_zip_code)
end
def validate_previous_zip_code
validate_zip_code(:previous_zip_code)
end
def validate_zip_code(field)
errors.add(field, :inclusion) if errors.on(field).nil? && !GeoState.valid_zip_code?(send(field))
end
validates_presence_of :home_zip_code
validates_format_of :home_zip_code, :with => /^d{5}(-d{4})?$/, :allow_blank => true
validate :validate_home_zip_code
validates_presence_of :mailing_zip_code
validates_format_of :mailing_zip_code, :with => /^d{5}(-d{4})?$/, :allow_blank => true
validate :validate_mailing_zip_code
validates_presence_of :previous_zip_code
validates_format_of :previous_zip_code, :with => /^d{5}(-d{4})?$/, :allow_blank => true
validate :validate_previous_zip_code
That looks very wet to me. (WET == “Write Every Time”) But it’s not too hard to dry this up using just a tiny bit of knowledge of how ActiveRecord validations work.
Wouldn’t it be nice to get rid of those three different validation methods and just have one thing to use to validate the zip code is real?
def self.validates_zip_code(*attr_names)
configuration = { }
configuration.update(attr_names.extract_options!)
validates_each(attr_names, configuration) do |record, attr_name, value|
if record.errors.on(attr_name).nil? && !GeoState.valid_zip_code?(record.send(attr_name))
record.errors.add(attr_name, :invalid_zip, :default => configuration[:message], :value => value)
end
end
end
validates_presence_of :home_zip_code
validates_format_of :home_zip_code, :with => /^d{5}(-d{4})?$/, :allow_blank => true
validates_zip_code :home_zip_code
validates_presence_of :mailing_zip_code
validates_format_of :mailing_zip_code, :with => /^d{5}(-d{4})?$/, :allow_blank => true
validates_zip_code :mailing_zip_code
validates_presence_of :previous_zip_code
validates_format_of :previous_zip_code, :with => /^d{5}(-d{4})?$/, :allow_blank => true
validates_zip_code :previous_zip_code
That’s a bit better. Before we had three custom validation methods that did all the same work using an extracted method. We have replaced those three methods with a single custom class-level validation, validates_zip_code, which validates a zip code attribute by name. It’s used just like any other standard validation, and as many times as you want. Nice and dry! The implementation of validates_zip_code is modeled on some of the standard validation methods found in ActiveRecord’s Validations module (in 'active_record/validations.rb'). Take a look at that code for more examples of how you might write your own custom validations.
Now that’s a good start, but we can do even better. We can take the custom validation and fold in the other validations we do on each zip code.
def self.validates_zip_code(*attr_names)
configuration = { }
configuration.update(attr_names.extract_options!)
validates_presence_of(attr_names, configuration)
validates_format_of(attr_names, configuration.merge(:with => /^d{5}(-d{4})?$/, :allow_blank => true));
validates_each(attr_names, configuration) do |record, attr_name, value|
if record.errors.on(attr_name).nil? && !GeoState.valid_zip_code?(record.send(attr_name))
record.errors.add(attr_name, :invalid_zip, :default => configuration[:message], :value => value)
end
end
end
validates_zip_code :home_zip_code
validates_zip_code :mailing_zip_code
validates_zip_code :previous_zip_code
This example was extracted from the version history of the code for a project I worked on recently. It follows the very steps we took to refactor the validations step by step. I know that Jeff Dean has contributed a cool validator object enhancement that will be appearing in Rails 3, but this approach works out of the box with the current version of Rails, so you don’t have to turn blue holding your breath.
Final note: If you’re wondering why we validate presence and then allow blank on the following validations, it’s so that the user only gets one error message at a time.
Refactoring a dead horse
A while back I made the point that the HasOneThroughAssociation class in Rails shouldn’t be a subclass of HasManyThroughAssociation. I also submitted a Rails patch in which I changed the superclass of HasOneThroughAssociation from HasManyThroughAssociation to HasOneAssociation and moved the shared Through functionality into a module. Despite support from the teeming millions, Rails core team member Pratik rejected the patch for being “just a refactoring.”
Despondent, I brought the issue up here at Pivotal a few months later, after the Release of Rails 2.3. Nate added a callout for support for the ticket to the daily Pivotal standup blog. The response was heartwarming (thanks to all who added a +1), but resulted only in Pratik removing himself from the ticket (I assume so he would stop getting comment notifications).
All is not lost, however. At RailsConf last week, Jeff Dean — Pivot, raconteur, international playboy, and refactoring paladin — called out the Rails core team regarding their stance on refactorings. The response: bullish (sort of).
Taking into account the responses from the Rails core team, I’ve done the following:
- Recreated the patch for current Rails edge. Fortunately, I had to make only one small change to the patch from six months ago.
- Ran a search on GitHub for any code using the HasOneThroughAssociation class (thanks to Jeff for the idea). As far as I can tell no code outside of Rails depends on the implementation of that class.
- Added Jeremy Kemper as the owner of the Rails ticket. He was the most immediately supportive of refactoring patches, so hopefully he’ll advocate for this one.
We’ll see what happens.
