74

One of the methods that I commonly use in our codebase is misspelled (and it predated me).

This really irritates me not simply because it is mispelled but more importantly it makes me ALWAYS get the name wrong the first time I type it (and then I have to remember "Oh, right, it should be mispelled to this...")

I'm making a few changes around the original method. Should I take the opportunity to just rename the freaking method?

Improve this question
13
  • 12
    Can you rename it? If it's used by code you don't control, you need to justify the backwards compatibility break.
    – user7043
    Commented Jun 11, 2014 at 16:59
  • 17
    *misspelled. And you'll have to change it everywhere that method gets called.
    – JohnP
    Commented Jun 11, 2014 at 17:01
  • 2
    *misspelt... or maybe a different spelling?
    – HorusKol
    Commented Jun 11, 2014 at 23:48
  • 2
    @JohnP There were Three, now One is fixed and Two are still incorrect. Coincidentally suits OP's name quite nicely. :)
    – Disillusioned
    Commented Jun 12, 2014 at 12:47
  • 3
    We must allow nothing to be incorrectly pelled!
    – Magus
    Commented Jun 12, 2014 at 16:30

6 Answers 6

Reset to default
139

Should I take the opportunity to just rename the freaking method?

Absolutely.

That said, if your code has been released as an API, you should also generally leave the misspelled method and have it forward to the correctly named method (marking it Obsolete if your language supports such things).

Improve this answer
14
  • 34
    The idea of "forward to the correctly named method" is simple and brilliant. Interesting link on the broken windows theory, as well.
    – dev_feed
    Commented Jun 11, 2014 at 21:03
  • 2
    Note that if this is part of a public API, this may not be a backwards compatible change (depending on language). This is not as unlikely as it may sound.. if I had to inherit from an existing API with a misspelt name I'd definitely fix that.
    – Voo
    Commented Jun 11, 2014 at 22:58
  • 10
    @voo - eh? What language would make adding a new method (and changing the implementation of one to do the same exact behavior) be not backwards compatible?
    – Telastyn
    Commented Jun 11, 2014 at 23:29
  • 3
    @Telastyn it can be tricky sometimes to add methods to say web services. Some clients balk at changing WSDLs for example, suddenly refuse to talk to the server. That's an implementation problem in the client, but if the client is an important one you don't want to upset it can very well prevent you from changing your interface.
    – jwenting
    Commented Jun 12, 2014 at 7:43
  • 17
    @Telastyn If the misspelt method name is on an interface (as in interface type used in Delphi/Java/C#), then adding a correctly spelt version of the name will probably break all existing implementations of said interface.
    – Disillusioned
    Commented Jun 12, 2014 at 12:53
51

There are cases where you should avoid doing such refactorings:

  1. If the method is used in a public interface. A canonical example is the misspelling of referrer in HTTP referer, the wrong spelling being kept, because changing the spelling now would have too many repercussions.

  2. If the code base is not covered by any tests. Any refactoring should be done on tested code in order to be able to do regression testing. Refactoring the code base which is not under tests is particularly risky. If you do have a lot of time, start by adding tests; if you work under time pressure, taking a risk of introducing subtle bugs is not the best thing to do if you want to ship on time.

  3. If the method could be used in an unusual way, which makes its uses practically impossible to find (through Ctrl+F or by an automated refactoring tool). For example, in C#, a method can be called through Reflection, making Rename dialog of Visual Studio ineffective. In JavaScript, the function called inside eval() is difficult to find as well. In PHP, variable variables could cause issues.

  4. If the size of the project is huge and the method could be used by other teams. This is similar to the first point, i.e. the interface you provide to other teams can be considered a public interface.

  5. If you deal with a life-critical project. Chances are, the misspelling is not too important to justify a few months of paperwork in order to change the name of the method and ensure it won't cause any patient to receive ten times the authorized radiation or any shuttle to miscalculate its speed.

In any other situation, feel free to rename the method.

Improve this answer
13
  • 1
    +1 for the comment mentioning Reflection, it's bitten me more than once.
    – DaveShaw
    Commented Jun 11, 2014 at 21:37
  • 34
    If your life-critical project is fragile enough that the slightest refactoring is likely to kill someone, it's fragile enough that no one should trust it with their lives. How are you ever going to be confident that your new feature or streamlined user interface didn't introduce life-threatening bugs if you can't even rename a method?
    – user2357112
    Commented Jun 12, 2014 at 6:30
  • 3
    Similarly, if one changed method name causes several extra months of paperwork (rather than, say, mostly getting taken care of by the paperwork for all the other stuff that's changing at the same time), then the programming-to-paperwork balance is skewed by several orders of magnitude, and it's impossible to get any major improvement done without changing the system.
    – user2357112
    Commented Jun 12, 2014 at 7:16
  • 8
    @user2357112: I never said that the slightest refactoring is likely to kill someone. It's not about killing someone, but about making everything possible to mitigate the remaining 0.001% risk of having a bug. This requires formal proff. This requires several layers of testing. This requires formalism. This forbids "I wanna quickly rename this method, hopefully it would work!" behavior. Life-critical projects use techniques which would be considered a complete waste of time and money for any business app. That's why they are so reliable (and expensive).
    – Arseni Mourzenko
    Commented Jun 12, 2014 at 7:41
  • 5
    @user2357112: as MainMa pointed it out, this is not about casual business apps. It's about special kind of software that are extensively tested/verified. What if the method is called by reflection somewhere? What if some pre/post compiling tool does something with it? What about the documentation? What about other teams using it? Do they use reflection? What if... Real life can be quite complex sometimes. And sometimes it is better to leave a method name untouched rather than verifying if there is any consequences in a bullet-proof manner.
    – dagnelies
    Commented Jun 12, 2014 at 8:55
30

I've done this a few months ago (for different reasons). The steps I took (the language was Perl):

  1. Rename the method. Alias the old name to the new name (this should not break any code, as the method can be called by either name).

  2. Inform the rest of the developers about the name change and why, telling them to use the new name from now on.

  3. Grep the code base for the old name, fix any occurrences.

  4. Log any uses of the old name (using the old name should still work at this time). Fix those cases.

  5. Wait (while doing 4.), until no more entries appear in the log.

  6. Break the alias. Make a method using the old name that throws a fatal exception with a message about the name change.

  7. After some time, remove the method with the old name.

    Of course, your mileage will vary.

Improve this answer
2
  • One improvement -- don't rely on grep to find users of the old name, also log inside the forwarder.
    – Ben Voigt
    Commented Jun 13, 2014 at 21:08
  • @BenVoigt Isn't that what #4 does?
    – Bob
    Commented Jun 14, 2014 at 19:47
6

A good way to not break any existing code would be to chain the new method name to the old in a such as 

private void MyNewMethodName()
{
    TheOldMethodName();
}

and then mark the old method as obsolete (if your language support this). This way any existing code will still work and you can gradually take out all the old spelling mistake out of your code base. Eventually you could even copy/paste the method body into the new method and delete the old one.

/Edit As ivo said in the comment : An even better thing to do would be to move the code from TheOldMethodName into the MyNewMethodName and call the new method from the old one. This one would also have the advantage to help the dev to get their head around where the code belong to.

Improve this answer
2
  • 2
    I agree with your method; I would do the same. It's the most sane, clear and secure method of refactoring. What also might be a nice way is to move the code from the old method into the new method and make the old one use the new method. When the codebase is a few iterations down the timeline you only need to remove the old deprecated method.
    – Ivo Limmen
    Commented Jun 13, 2014 at 9:07
  • 1
    @IvoLimmen That's a really good suggestion. This way people get even more their head around using the new method and this would remove a warning from the obsolete call to the old method from within the new one. I will add this to my answer.
    – Rémi
    Commented Jun 13, 2014 at 11:36
1

Renaming the method:

  • Do it through refactoring lest you have more work to do than you want
  • If your IDE supports auto-completion, use it when referencing that method

Those are two options you could go for. I would prefer auto-completion (ex. Eclipse IDE) and not need to type out the method name. Going for the rename; just make sure you find out what calls that method and change direct references in each place. Refactoring will be your friend for that but be most careful when doing so.

Improve this answer
0

I would generally recommend yes, rename it.

Other answers here have listed good reasons why you might not want to renamed it though, so if you find yourself in one such situation, you can create a new method with the proper name and implementation, and change the old method to call the new method. Then mark the old one as deprecated if your language supports it.

Improve this answer

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