3

SonarQube is raising an issue on my string formatter use:

Format specifiers should be used instead of string concatenation.

I have used below java code to add padding 0 in the number.

int paddingLength = seqLength - seqNoLength;
String.format("%0" + paddingLength + "d", seqNo);
G. Ann - SonarSource Team
  • 22,346
  • 4
  • 40
  • 76
Shiladittya Chakraborty
  • 4,270
  • 8
  • 45
  • 94
  • 1
    Because format strings are interpreted at runtime, rather than validated by the Java compiler, they can contain errors that lead to unexpected behavior or runtime errors. – Akshay Jun 08 '17 at 07:44
  • Any alternative way to ignore this sonar issue? – Shiladittya Chakraborty Jun 08 '17 at 08:08
  • Format the formatString ? – benzonico Jun 08 '17 at 08:12
  • Yes. Any other best way to achieve the same – Shiladittya Chakraborty Jun 08 '17 at 08:19
  • From [this](https://stackoverflow.com/questions/2550123/java-printf-using-variable-field-size) it appears that what you are doing is fine. – Klitos Kyriacou Jun 13 '17 at 10:52
  • @Novaterata it would be ok for SonarQube to give this warning if it was something like `String.format("%s", "N: " + seqNo)` but in this case SonarQube is overzealous. You can't do much better than `String.format("%0" + paddingLength + "d", seqNo)`. The alternative, `String.format(String.format("%%0%dd", paddingLength), seqNo)`, is much worse than the original. SonarQube ought to suppress this warning when concatenation is used for format specifiers. – Klitos Kyriacou Jun 13 '17 at 11:13
  • @KlitosKyriacou You're right. I totally missed that they were actually concatenating the formatting – Novaterata Jun 13 '17 at 12:22

1 Answers1

4

The issue is raised because you're passing a concatenated string to your formatter. If you're going to use string formatting, then use string formatting:

int paddingLength = seqLength - seqNoLength;
String fmt = String.format("%%0%dd", paddingLength);
String.format(fmt, seqNo);

Or even 

int paddingLength = seqLength - seqNoLength;
String.format(String.format("%%0%dd", paddingLength), seqNo);

Although the first version is far more readable.

mjuarez
  • 16,372
  • 11
  • 56
  • 73
G. Ann - SonarSource Team
  • 22,346
  • 4
  • 40
  • 76
  • Thanks @Novaterata. Fixed – G. Ann - SonarSource Team Jun 08 '17 at 12:50
  • I've just re-examined the documentation and tried it. It doesn't seem you can do this. – Klitos Kyriacou Jun 13 '17 at 10:24
  • 1
    Misunderstood the intent originally @KlitosKyriacou. See my change. – G. Ann - SonarSource Team Jun 13 '17 at 11:56
  • Thanks for the comment. Your changed code works well. However, I'm not convinced that this is clearer than, or has any advantage over, the original code that used concatenation. I presume the SonarQube warning exists because there are advantages to String.format compared to concatenation when it comes to e.g. internationalization. However, this applies mostly to text that is ultimately going to be output by the application. On the other hand, format syntax is not going to be output at all; it should be seen as part of the program code, and not be subject to this warning. – Klitos Kyriacou Jun 13 '17 at 14:51
  • @KlitosKyriacou if this were your code, I'd advise you to mark the issue "Won't Fix". As it stands tho I'm not the only one who missed the intent of the original code. So off-hand I'd say my first code snippet is superior merely because it makes the intent crystal clear. – G. Ann - SonarSource Team Jun 13 '17 at 15:00