Don’t do this:
<% if File.exists?(model.path) %>
...
<% end %>
Do this instead:
<% if model.file_exists? %>
...
<% end %>
You’re reading Signal v. Noise, a publication about the web by Basecamp since 1999. Happy !
Don’t do this:
<% if File.exists?(model.path) %>
...
<% end %>
Do this instead:
<% if model.file_exists? %>
...
<% end %>
Anonymous Nick
on 28 Sep 09Aside from saving a few keystrokes, what’s the benefit?
Matt Pennig
on 28 Sep 09Makes total sense. Cleans up the view, and allows the model to cache the result (if necessary).
Matt B
on 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
on 28 Sep 09One could easily argue that doing that adds unnecessary knowledge to the wrong class.
Jamis
on 28 Sep 09Fair 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
on 28 Sep 09I 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
on 28 Sep 09Makes sense. Thank you for the explanation!
Matt B
on 28 Sep 09Jamis thanks for the explanation.
jon
on 28 Sep 09You 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
on 28 Sep 09I 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
on 29 Sep 09I 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
on 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
on 30 Sep 09Chuck +1
In fact, IMHO, Chuck’s is the only correct answer.
HappyProgrammer
on 05 Oct 09While 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.
This discussion is closed.