• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

A question about magic strings

 
Ranch Hand
Posts: 146
Eclipse IDE Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
My coworker took a look at some code I wrote today, and pointed out that I have used several 'magic literals'
Especially, he objected to code that looked like this:

#1 What do you think about this example? Should these Strings be located outside of the method as statics?
#2 I think the point is that local variables should only be used where they contain arguments from the method itself.
(An exception might be a boolean variable to return the result of a test) Does this sound right?
 
Marshal
Posts: 79179
377
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I would keep those variables as local variables. I think he may be overreacting a bit, but others will doubtless have different opinions.
 
Billy Sclater
Ranch Hand
Posts: 146
Eclipse IDE Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Thanks for the feedback. Most of the local variables used (in the case of my code) are used only once, literally locally, and their values aren't really subject to change. So it makes sense to me too to keep them local!
 
Campbell Ritchie
Marshal
Posts: 79179
377
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
You're welcome Bit surprised nobody disagreed with me.
 
Billy Sclater
Ranch Hand
Posts: 146
Eclipse IDE Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Yeh, I was expecting a flood of disagreements too! Maybe everyone agrees with you !
 
author
Posts: 23951
142
jQuery Eclipse IDE Firefox Browser VI Editor C++ Chrome Java Linux Windows
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Well, that's what the string pool was designed for ... it is not like new string objects are being created with each method call, nor are they discarded when the local variables go out of scope.

Henry
 
Campbell Ritchie
Marshal
Posts: 79179
377
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Billy Sclater wrote: . . . Maybe everyone agrees with you !

Maybe. It has never happened before though
 
Sheriff
Posts: 67746
173
Mac Mac OS X IntelliJ IDE jQuery TypeScript Java iOS
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
If the format string were used a number of times, I'd consider making it a class constant so that the possibility of a typo is minimized. But in the snippet you posted, you'd doing so just because; and "just because" isn't usually that much of a justification.

The s string isn't used at all so it's hard to have any opinions on it.
 
Billy Sclater
Ranch Hand
Posts: 146
Eclipse IDE Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
How about this:
 
Bear Bibeault
Sheriff
Posts: 67746
173
Mac Mac OS X IntelliJ IDE jQuery TypeScript Java iOS
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Without a larger content it's hard to say, but I usually put file paths (even those that are not supposed to change -- famous last words) in properties files.

The rule of thumb I use: will the code be clearer? Easier to maintain? More robust?

Embedding strings in the code can make them difficult to see/find, so extracting them to a constant can make them more visible, and of course, allow them to be shared. But whether to hoist the string literal to a class constant or not is a judgement call. Things like single-use format strings usually don't make the cut for me.

If you are looking for hard rules: as usual, there ain't none.
 
Billy Sclater
Ranch Hand
Posts: 146
Eclipse IDE Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Thanks everybody, that's something for me to think about
 
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Billy Sclater wrote:Thanks everybody, that's something for me to think about


Actually, I'm more interested in your colleague's use of the word "magic".

Your code is an example of what we used to call "hard-coding", which means that it does only one thing. Now there's nothing wrong with "one thing" when it comes to responsibility, but not when to comes to values, or things that might be equally easily supplied as parameters, because it makes code "brittle" - by which I mean that if you want it to do something else, you have to change the code. And that's generally not a good idea.

Bear's suggestion is just one way to go in the case of filenames, but in the case of a copy utility, it might be even better to simply supply them at runtime - ie, make them part of the 'args' you supply to your main() method.

HIH

Winston
 
Bartender
Posts: 689
17
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Normally I would say that literals should be compile time constants. However I think there is a very strong case that format strings should always be local string literals defined as part of the call to String.format(...).

The format string is not just a value, it is functional. More than that, it determines what parameters are required as part of the format. If you get the number or type of the parameters wrong you get a RuntimeException, not a compiler warning.

If you hide the content of the string from the reader at the point String.format is called then it makes it very easy to make mistakes that won't be picked up until Runtime.

If the same string format literal is used in multiple places then the format should have its own method to minimise the chance of errors.

All that being said, FindBugs is very good at spotting mistakes when calling String.format(...).
 
Billy Sclater
Ranch Hand
Posts: 146
Eclipse IDE Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Actually, I like Bear's properties file idea for storing file paths.

Ok, I'm going to be decisive!! I think I'll keep variables as local if
a)the variable is only used once in one method of the class, and
b)it's value is not likely to need changing (for example Bear mentioned putting file paths that might change into a properties file).

And of course there are probably some exceptions to this rule, but that it was (I personally think) generally sounds workable!!

Thanks (:
 
Campbell Ritchie
Marshal
Posts: 79179
377
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Rule of thumb: the narrower the scope you can keep variables in the better.
Obviously there are some things which have to be fields or even global constants, but try making everything else local variables first.
 
With a little knowledge, a cast iron skillet is non-stick and lasts a lifetime.
reply
    Bookmark Topic Watch Topic
  • New Topic