Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

My two cents on what widget.versions.last.next_version should return. #124

Closed
joshuapinter opened this issue Jan 19, 2012 · 8 comments
Closed

Comments

@joshuapinter
Copy link

Awesome gem Andy.

Just reading through and noticed your call for opinions: "As an aside, I'm undecided about whether widget.versions.last.next_version should return nil or self (i.e. widget). Let me know if you have a view."

I would say return self, or widget, when this happens. That seems more intuitive, even if it masks an "incorrect" action.

Cheers,

Josh

@airblade
Copy link
Collaborator

airblade commented Feb 7, 2012

Thanks for the feedback, Josh. Much appreciated!

@ndbroadbent
Copy link

Hi, just my 2c - I would personally vote for nil, since I imagine using it as a test to see if there are any further versions.
It would be especially useful when building a UI to browse versions.

I could imagine writing an ERb view like this:

<% if version.next_version %>
   <%= link_to "Next version", url_for_version(version.next_version) %>
<% else %>
   <p>This is the current version.</p>
<% end %>

Cheers

@joshuapinter
Copy link
Author

Good point ndbroadbent. I could see it being used like that as well but you're really asking a question at that point. How about something like:

<% if version.next_version? %>

@ndbroadbent
Copy link

That's true, it's more of a question. In that case, I would prefer the inverse:

<% unless version.current? %>

I don't know if that resolves it though... My gut feeling is still telling me that it might be better to debug an undefined method for nil:NilClass, than for that same bug to go unnoticed.

@joshuapinter
Copy link
Author

I'm convinced. I think return nil would be the safest bet.

Adding a current? or latest? would be fantastic as well.

@mckramer
Copy link

Isn't that what live? does already?

@hazah
Copy link

hazah commented Apr 25, 2012

Semantically, the live object represents the very last version of that object. It is the "Live Version".

@batter
Copy link
Collaborator

batter commented Feb 28, 2013

Hey guys, I know I'm a little late to the conversation, but I agree with what @hazah said, and in the latest release (2.7.1), I altered the next_version method so that it will return the self (the live version).

If you want to know whether the object returned by next_version is actually the live version or not you can just use: widget.next_version.live?.

See #200 for my rationale, as well as the commit where the change was made. If someone has strong objections to this, we can revisit this, but I truly feel that the live version should be considered a version, and as such should not be ignored by the next_version method.

@batter batter closed this as completed Feb 28, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants