Hacker News new | past | comments | ask | show | jobs | submit login

But it's not "test-specific functionally", is it? The underlying object has a published_at attribute. Don't you think it means something?

The reality, then, is that we don't publish things, but publish things as of certain times. Only by putting the concept of publication time in the interface do we communicate the true semantics of publication.

To look at it another way, if you call the publish! method without understanding how publication time affects the thing you're publishing, aren't you missing something important?

So, to make the semantics of publication clear, the publish! method ought to have a publication-time parameter of some sort. (But it shouldn't be called current_time, as that name doesn't reflect the actual semantics of publication. We don't want to suggest, for example, that it requires time travel to schedule things for future publication.)




It's "test-specific" if the only time you ever specify that parameter is in a test. You're right that published_at is an attribute that can be set, but if you just wanted to set published_at to an arbitrary timestamp, Rails already provides that functionality for you: update_attribute :published_at, some_time.

I agree with everything else you've said around the semantics of publishing something, but those are implementation details of the application at hand. Having a #publish_at!(publish_time) method may or may not be what users need in this application, but the original example (adding a parameter to make testing cleaner) seems like a clear case of writing to the test to me.


My point is that, in this case, it is not possible to understand what it means to "publish" something without understanding the implications of its publish_at time. That is, it's not an "implementation detail" but an essential part of the semantics of publication.

So, yes, if the original author left the concept of publication time out of the publish! method, that was probably a mistake. Likewise, adding a current_time parameter to that interface for the purpose of injection at testing time was also a mistake.

But my point is that both of these mistakes are downstream consequences of failing to recognize that, in this application, the published_at time is an essential part of the publication semantics and ought to be clearly expressed in the interface for publishing things. Had it been been expressed, there would have been no problem during testing because the interface would already work for any given publish_at time and not be hard-coded for the single (although common) case of publishing at now.

In sum, a better interface all around would have been to have the publish! method take a publish_at time that defaults to the current time. You can't really make the interface "simpler" by leaving it out of the method interface: it's essential to publishing something, so it must be there somewhere, if not spelled out clearly in the interface, then hard-coded to some value internally, where all it can do is cause trouble.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: