Learn Ruby Science – Book in progress

https://learn.thoughtbot.com/products/13-ruby-science

by duck • 14 years ago

14 3 14 years ago

learn.thoughtbot.com

I respect Thoughtbot and their contributions to the Ruby community, but the section on smells glosses over way too much.

Here is the link to the sample: https://learn.thoughtbot.com/ruby-science-sample.pdf

And here are some counterpoints:

Long methods are not as easily testable, but overly short methods can make code much harder to read and follow and actually increase complexity, because now there are many more entry points, leading to excessive nil checking, etc. for those practicing paranoid TDD.

Even the developers of Rails are not sold on the promise of the null object pattern, and as it states as almost an afterthought that "it can lead to confusion" (paraphrased). If you are a developer, especially one that is a refugee from a more "type-safe" language, you are more likely to assume that:

an_object.a_method

Will fail if an_object is nil/null. By making an_object a null object, then you can end up with is_a?(NullSomething) when other programmers are using your API, defeating the whole purpose of trying to avoid additional try's and other checks in the code. It can be a bad code smell to use null object pattern in many situations.

Case statements can be a bad code smell, just as is_a?(...), but not in every situation. Often they can be a clear and concise way to get the job done. In Java the switch statement was ridiculed, only to produce a whole generation of Java developers that either had chained if's or would create 3x the code just to introduce a pattern for simple logic!

So, if the rest of this book is like this, please take it with a grain of salt. It is a reference for another way of thinking about things, but if you are going to do that, read "Design Patterns" by the GoF and "Refactoring" by Martin Fowler also. And then forget all of that and try to write DRY Ruby code in as few lines as possible without tests to get a feel for that. Then give both up and seek a middle ground. In this way, you will be like Bhudda. Neither conservative "tests, patterns, and refactoring is the only way or you are shunned" nor liberal "it doesn't matter how you write it, just write what you like" are the ways to enlightenment; it is the way between them.

harlow• 14 years ago

> Overly short methods can make code much harder to read and follow and actually increase complexity, because now there are many more entry points.

Ideally there wouldn't be any more entry points. If we create multiple short well-named private methods they should make it easier to follow your code. Side benefit of well-named methods is they can act as simple documentation for readers.

> Case statements can be a bad code smell, just as is_a?(...), but not in every situation.

More often than not when doing a code review a case statement will catch my eye. Often they are OK, but more often than not they can be avoided.

> Read "Design Patterns" by the GoF and "Refactoring" by Martin Fowler also.

So important to read these. They are great for all developers. The idea behind Ruby Science is to show how to use these techniques in a Ruby on Rails application. With real code, and real refactoring (along with code, and a history of git Commits to show the exact changes we're making).

jferris• 14 years ago

Agreed on method extraction; overuse will just introduce more hoops to jump through when trying to understand the code. Unless you're moving methods to other classes, though, you can avoid adding entry points to the API by extracting methods as private. We use private methods to reveal intent, break down large methods, and reuse behavior within a class.

Also agreed on Null Object. I tried to make it pretty clear in the chapter that it's not a one-size-fits-all kind of solution and must be used with care. I strongly advise against using is_a?, ===, or case statements to check for null objects.

You can define present? to be false on your null class if you NEED to check for a null object, but the pattern is only useful when the higher level code (views, etc) don't need to care about whether or not the object is null. If you have to add present? checks to everything, I'd advise against using null objects.

The book is also trying to highlight that "middle way" you're talking about. Each chapter tries to outline the benefits and drawbacks of using a particular approach. We've stressed that you don't need to "fix" every code smell you find, and that the solution depends on the specifics of an application. The book is designed to give developers a better understanding of the methods for finding and fixing problems, and the solution chapters try to give the developer the ability to decide what the best solution is for a specific situation, rather than trying to achieve an "if x then you always y" kind of approach which doesn't work in the real world.

Learn Ruby Science – Book in progress

Related Stories