When is it a good use of time to refactor string literals?

Question

I'm starting on a project where strings are written into the code most of the time. Many strings might only be used in a few places but some strings are common throughout many pages.

Is it a good use of my time to refactor the literals into constants being that the app is pretty well established and runs well? What would be the long-term benefits to doing so?

Answer 1

Only if this project needs to be supported into the future is this a good use of time. If you will be regularly maintaining/expanding this system; however, this is a great idea.

1) There is a large degree of risk associated with string literals as a single misspelling can usually only be detected at run time. The reduced risk of run time errors is a serious advantage as they can be embarrassing/frustrating.

2) Also, should they ever need to be changed, for example when they are used to reference another system (like table names, server names etc) they can be very difficult to update when those other system names change. Centralize them and it's a trivial issue.

Answer 2

One common thing to consider would be i18n. If you (or your muckity-mucks) ever want to sell your product in Mexico or France (etc.) you're going to appreciate having those string literals not littered throughout the code base.

EDIT: I realize this doesn't directly answer your question, so I'm voting up some of the other answers re: rule of three, and the like. I understand you're talking about an existing code base, so it's a little late to talk about incorporating i18n from the start. It's so easy to do when you're in the habit from the start.

Answer 3

I like to apply the rule of three when refactoring. If it happens three or more times, then the code needs to be updated.

Answer 4

If a string is used in more than one place, refactor it. If it is only used in one place, leave it alone.

Answer 5

If you've refactored out all your common strings, it makes it easier to internationalize/translate them. It's even easier if they're all in properties files, or whatever your language equivalent is.

Answer 6

Is it a good use of my time to refactor the literals into constants being that the app is pretty well established and runs well?

No, you better leave it like it is.

What would be the long-term benefits to doing so?

If no one ever touch that code, the benefits are none.

What you can do, however is avoid adding new literals. But I would pretty much leave existing the way they are.

You could probably refactor them in your free to sleep better.

Probably there are some other bugs already that need your attention. Fix those instead.

Finally, if you manage you add "refactoring" to your task list, go ahead!!!

Answer 7

I agree with JMD, just keep in mind that there is more to i18n than changing strings(currencies, UI must be adpated to Right-to-left languages etc)

Even if you don´t wish to 18n your application it would be useful to refactor your strings, since that string that is used today only once, maybe reused tomorrow several times, and if it´s hardcoded you may be not aware of it and star replicating string all over the place.

Answer 8

Best let sleeping dogs lie. If you need to change a string that's used eighteen-lumpy times, yeah, go ahead and turn it into a constant somewhere. If you find yourself working in a module that has a string that could be constant-ize, do it if you feel like it. But going through the whole app changing all strings to constants... that should be on the very bottom of the to-do list.