18

I currently manage a library which has a lot of public usage, and I had a question about semantic versioning. I want to refactor one fairly important part of the library which is implemented incorrectly - and has always been implemented incorrectly. But doing this would mean changes to the public API, which is a major decision.

The change I want to make revolves around how iterators are used. Currently, users have to do this:

while ($element = $iterator->next()) {
   // ...
}

Which is incorrect, at least in PHP's native Iterator interface. I want to replace with this:

while ($iterator->valid()) {
   $element = $iterator->current();
   // ...
   $iterator->next();
}

which is analogous to:

foreach ($iterator as $element) {
    // ...
}

If you look at Tom's guide to semantic versioning, he clearly states that any changes to the public API (i.e. those which are not backward compatible), should justify a major release. So the library would jump from 1.7.3 to 2.0.0 which, for me, is a step too far. We're only talking about one feature being fixed.

I do have plans to eventually release 2.0.0, but I thought this was when you completely rewrote the library and implemented numerous public API changes. Does the introduction of this refactoring warrant a major version release? I really can't see how it does - I feel more comfortable releasing it as 1.8.0 or 1.7.4. Does anybody have some advice?

  • What prevents you from keeping backward compatibility? – mouviciel Nov 28 '13 at 13:41
  • At the moment, the next() method is used to retrieve the current element AND move the internal pointer forward. Which is wrong. next() should move the pointer, and current() is used to retrieve... – hohner Nov 28 '13 at 13:44
  • 6
    so in the new version people shouldn't care about the return value of next() only that is moves the pointer, this really doesn't break compatibility – ratchet freak Nov 28 '13 at 14:28
29

You hesitate because you don't want to make semantic versioning, you want to make "advertisement supporting versioning". You expect a version number "2.0" to tell the world that you have a bunch of new cool features in your library now, not that you changed the API. That's ok (many software companies and/or developers do that). IMHO you have the following options:

  • stick to semantic versioning and live with the fact that you have to change the version number to 2.0.0
  • change your versioning scheme to 4 numbers. "1.1.7.3" is your version now, "1.2.0.0" the next one after changing the API, and "2.0.0.0" the first one of the "completely new 2.x product family"
  • make your fix backwards compatible (so don't change the functionality of next, just add the valid and current functions). Then you can use "1.8.0" as the next version number. If you think changing the behaviour of next is really important, do it in 2.0.0.
  • As much as the last option would be the perfect solution: you can't ask next() to keep on doing what it's doing. To implement the functionality correctly, it has to do something differently. So if I make it backwards compatible - the new functionality/fix will also be wrong, and undermine the entire point of the change. – hohner Nov 28 '13 at 13:46
  • 2
    The broader suggestion that you make in your third bullet (making the fix backwards compatible) is a good one to consider. It may not work in this particular case, but the overall technique is worth considering. The function ends up being more complex but this may be a workable route. – GlenH7 Nov 28 '13 at 13:57
  • Thanks to everyone: if I could accept two I would. I ended up hacking the new next() method to do all the new functionality, plus what was needed to make backwards compatible. It feels kind of horrible to have to tarnish new functionality like this, but hey ho. – hohner Nov 28 '13 at 14:05
  • 10
    @hohner: Then now is also the time to document the old behaviour as deprecated, so you can remove it in 2.0.0. – Jan Fabry Nov 28 '13 at 15:18
7

Stick with Tom's guide to semantic versioning.

Any significant change to a public API must be done at either of the two points:

  1. Never
  2. At a major release update

My vote, by the way, is for the first. But I acknowledge it's only appropriate for trifling things.

The problem is maintaining backwards compatibility and making sure you don't break things for the previous users of your API.

In essence, you are creating an indexing error for your users who are unaware of the change. Forcing a change like this forces all of your users to do the following:

  1. Code the fix to use the new approach
  2. Validate the fix and make sure it didn't break anything
  3. Ship new releases of their product to their end users

That can potentially be a lot of effort, especially when you consider just how few projects have test cases in place in order to validate changes like this. The amount of effort compounds when you consider the number of downstream users from your users who will also need to update their installations.

For something this small, I'd let it go and not bother with it.
If it really bothers you (which apparently it does or you wouldn't have asked) then I would do the following.

  1. Create the v2.0.0 branch in your code tree
  2. Make the first contribution to the v2.0.0 branch, which is this change
  3. Send out a preview Release Notes ahead of time broadcasting that the change is coming

And then be patient as it will take a while to accumulate other things that justify upgrading the version number to a new major release. The advanced notification (part 3) gives you time to receive feedback from end users to find out just how much of an impact that change is going to be.


An alternative solution is to add a new function that operates in the way that you want.

If you have foo() then you would create fooCorrect() in order to provide the fix but also fully preserve backwards compatibility. And at some point you can deprecate foo() to let others know not to use it.

The challenge there is that you'll find something else within fooCorrect() which necessitates it's update and you end up with fooCorrectedCorrect() or some other silly nonsense.

If you really want this fixed now, this alternative approach is probably the best route. Be aware of and wary of creating lots of extra functions this way as it makes the API harder to work with. And that awareness may be enough to prevent the worst of these types of problems.

But this might be the "least bad" approach to consider for something small.

  • I agree with you. The problem I face is that I want to completely rewrite the library for v2.0.0 (because there are a lot of these issues that need to be fixed); so I don't want such a small change as iterators to form the basis of this big change. So my options are either: ignore this bug, or fix the bug and put it into a new major version? – hohner Nov 28 '13 at 13:43
  • @hohner - updated answer to provide an alternative approach with creating new functions. Be mindful that lots of new, similarly named functions is almost as bad as changing the API itself. – GlenH7 Nov 28 '13 at 13:54
  • 3
    @hohner: Consistently wrong > inconsistently right in this case. The behavior still functions, it just isn't idiomatic. Consider that if you make this change you are breaking client code. To do this without warning will not be appreciated. – Phoshi Nov 28 '13 at 13:54
  • @GlenH7 In this case, using an alternatively named method will not work. PHP's native iterator relies on these methods (i.e. next() not nextCorrect()). I'll see if I can modify next() so it's backwards compatible AND works when implementing the Iterator interface. – hohner Nov 28 '13 at 13:56
  • 1
    @Phoshi You're spot on - I completely agree now. Now it's time to try and code the impossible :D – hohner Nov 28 '13 at 13:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.