Realized by Jamis on September 28 2009:
Don’t do this:
<% if File.exists?(model.path) %>
...
<% end %>
Do this instead:
<% if model.file_exists? %>
...
<% end %>
This is Signal vs. Noise, a weblog by 37signals about design, business, experience, simplicity, the web, culture, and more. Established 1999 in Chicago. Visit the Product Blog for more information on our products.
Jobs: |
See more on the Job Board. |
Looking for something? Visit the archives.
Who are 37signals?
Jason Fried, David Heinemeier Hansson, Sarah Hatter, Ryan Singer, Sam Stephenson, Jamie Dihiansan, and Michael Berger in Chicago, Matt Linderman in NYC, Mark Imbriaco in Wake Forest, North Carolina, Jeremy Kemper in Pasadena, California, Jeffrey Hardy in Vineland, Ontario, Joshua Sierles in Granada, Spain, Jason Zimdars in Oklahoma City, Craig Davey in Ottawa, Ontario, and Mr. Jamis Buck in Caldwell, Idaho.
37signals products
Getting Real: The book
The smarter, faster, easier way to build a successful web application. 171 page PDF, $19. Buy it now and download it today!
Basecamp project management
Everyone's favorite web-based project management tool. Easy, fast, elegant. Free trial.
Highrise simple CRM
Keep track of who your business talks to, what was said, and what to do next. Free trial.
Campfire group business chat
Simple and easy web-based group chat for business has finally arrived. Try it free today.
Backpack information manager
Gather your ideas, to-dos, notes, photos & files online. Set email and mobile reminders. Try free!
Writeboard collaborative writing
Write, share, revise, compare. Sharable web-based text documents with simple version control.
Ta-da List, to-do list manager
The web's simplest, fastest, and most useful to-do list manager. Get things done. Free, 10 second signup.
Ruby on Rails
For developers we present our powerful yet simple open-source web application framework. Make coding easier on yourself.
Got a web design project in mind? Find a web designer on Haystack. Browse by visual style, portfolio, budget, and geographic location.
Over 1 million people use 37signals' simple web-based software to collaborate on projects, track contacts, and organize their business with an intranet.
14 comments so far
Anonymous Nick 28 Sep 09
Aside from saving a few keystrokes, what’s the benefit?
Matt Pennig 28 Sep 09
Makes total sense. Cleans up the view, and allows the model to cache the result (if necessary).
Matt B 28 Sep 09
“Do X, not Y” is useless without an explanation of why.
Otherwise you’re just training people to blindly follow advice they found on the internet
Sean McArthur 28 Sep 09
One could easily argue that doing that adds unnecessary knowledge to the wrong class.
Jamis 28 Sep 09
Fair enough. Here’s why.
Imagine a scenario where you are storing user uploads on disk. You use File.exists? to check whether the file is present before you serve up a URL to it. Further, you wind up doing this in a few places: controllers, view helpers, views themselves.
Then, down the road, you make the decision to host your files somewhere else. Perhaps on S3, perhaps using a distributed filesystem like MogileFS, whatever. Suddenly, File.exists? is the wrong method. And you have liberally sprinkled calls to it all over your view and controller.
Reason #2 (and, really, the fundamental reason): File.exists? is an implementation detail. Your controllers and views should never care about implementation details. If a record references a file, only the record should know how and where the file is stored.
Chuck 28 Sep 09
I think Jamis is getting at the Law of Demeter.
This was one of the original principles of object-oriented design. Objects are meant to be like independent organisms that are told what you want them to do and asked questions about themselves. The first code sample treats the object more like a C struct than an actual autonomous object with its own set of behaviors.
Anonymous Nick 28 Sep 09
Makes sense. Thank you for the explanation!
Matt B 28 Sep 09
Jamis thanks for the explanation.
jon 28 Sep 09
You hear about the fat model skinny controller, but this is another variation on the theme, fatter model-thinner view. If you do any detailed web work you know that you typically have slightly different views or text based on if x=null, x=0, x=1 or x=n. Adopting fatter models helps ensure consistency, across views, but can also couple view testing so you better have good tests.
Brian Chiasson 28 Sep 09
I would go one further and not make your view know anything about a “file.” It could be as simple as a method rename, but it’s still some indicator of underlying detail. The file has a business meaning and your view shouldn’t care (except in the case of linking to it, but you can get around that) what type of whatever it is. Your scenario/explanation makes my case for me.
Anonymous Coward 29 Sep 09
I don’t really know ruby, but by your explanation, shouldn’t the method be called something like “model.exists?”, since as you point out, it might well be a lot of other things than a file on disk.
Jamis 29 Sep 09
@Anonymous, the problem with “model.exists?” is that its meaning is ambiguous. Does it mean that the database record exists? Or is it talking about some other non-database data that is associated with the record? In this case it’s the latter, but it’s not clear. Now, the term “file” is well understood, and even though we aren’t talking about a series of bytes on a local disk, we can still talk about “files” on S3 or in MogileFS or whatever. So I would argue that “model.file_exists?” is sufficient for our purposes, and has the added benefit of specificity. However, if the term “file” bothers you, you can always use some other synonym, like “asset”, or “attachment”, or even “external_data”.
Darren 30 Sep 09
Chuck +1
In fact, IMHO , Chuck’s is the only correct answer.
HappyProgrammer 05 Oct 09
While I think these kinds of observations are useful, the more I program, the more I believe that ultimately this kind of stuff really isn’t that big a deal.
More important – is the thing you’re making worth a damn? Really, that’s all that matters. Also, designing theoretical abstractions is the first step towards Architecture Astronautism that’s taken over Java. I see that the world of ruby isn’t immune.
Comments are closed