Though it might be trivial for someone, I find it a little inconvenient when someone formats the string while passing it as a parameter to a method. For e.g.

AddMessage( string.Format("{0} (" + Constants.Message1 + ")",
    string.Format("{0}" + FileExtensionPdf,

I have taken a simple example here, but it could become a little messy sometimes. I prefer to not do this and instead format my string before I pass them to the method. For me, this reduces readability.

I would like to know if there is a standard practice which goes against the above style. Since I am doing a code review, I am not sure if I should put it as a comment.

  • 2
    Never do or advocate something "because others do it". That is really all that "standard" or "practice" means. Do it because it provides a benefit over doing it differently. Formatting messages with substitutions like printf does greatly increases readability, so that is the reason you should be giving. Commented Jan 12, 2016 at 13:58
  • 1
    In the example code you are using both formatting and concatenation, which looks a bit strange. That's the first thing I would comment in a code review.
    – Torbjørn
    Commented Jan 12, 2016 at 14:10
  • 4
    C# 6 added string interpolation which greatly helps readability in string formatting: msdn.microsoft.com/en-us/library/dn961160.aspx
    – 17 of 26
    Commented Jan 12, 2016 at 14:11
  • 1
    There will always be devs who sacrifice readability for brevity. Some (mainly from non-C/C++ backgrounds) despise the ternary operator for example.
    – Robbie Dee
    Commented Jan 12, 2016 at 14:12
  • @17of26 - The link says service is unavailable. Commented Jan 12, 2016 at 14:28

3 Answers 3


I would probably format your example code something like this:

var fileName = Path.GetFileNameWithoutExtension(document.FileName);
    foo: string.Format("{0} ({1})", fileName, Constants.Message1),
    bar: string.Format("{0}{1}", fileName, FileExtensionPdf));

I think it's fine to format your input directly in the call as long as you do it in a readable way. Here I also use named arguments since it's two string arguments and I want to communicate clearly and avoid the stupid mistake of swapping those arguments.

Using a temporary varible for the file name with no extension is both an optimization of code and readability.

  • +1 Much better. The redundant code is removed, the author gets to keep their inline formatting and there are named arguments.
    – Robbie Dee
    Commented Jan 12, 2016 at 14:21
  • This looks much better as well as concise. You expanded it just enough make it cleaner and more readable. Commented Jan 12, 2016 at 14:31

I would simplify Torbjørn's answer with string interpolation:

var fileName = Path.GetFileNameWithoutExtension(document.FileName);
AddMessage($"{fileName} ({Constants.Message1})", $"{fileName}{FileExtensionPdf}");
  • 1
    I haven't started using C# 6 yet, but this is obviously very nice :)
    – Torbjørn
    Commented Jan 12, 2016 at 16:05
  • Me neither but certainly a bauble to look forward to...
    – Robbie Dee
    Commented Jan 12, 2016 at 16:20
  • it is neat, but i do feel that we lose something when we move away from the explicit use of the variables
    – Ewan
    Commented Jan 12, 2016 at 16:21
  • The explicit variables are still there!
    – 17 of 26
    Commented Jan 12, 2016 at 16:41

I agree expanding it out makes it more readable and maybe easier to debug. not sure I would raise it as a problem though.

const string msgFormat1 = "{0} ({1})";
const string msgFormat2 = "{0}.{1}";

string fileName = Path.GetFileNameWithoutExtension(document.FileName);

string msg1 = String.Format(

string msg2  = string.Format(

AddMessage(msg1, msg2);

edit : stole filename optimization from other answer

  • 2
    Creating msgFormat1 and msgFormat2 moves the formats further away from where they are used, making it harder to read now (in my opinion).
    – Torbjørn
    Commented Jan 12, 2016 at 14:20
  • I guess it depends somewhat on the rest of the code on whether one design is demonstrably better than another. ie are the formats used elsewhere/loaded in from config etc
    – Ewan
    Commented Jan 12, 2016 at 14:25
  • Yes, I agree. I will always inline the formats if I can though (if they are unique to this call to Format, if they are not dynamically loaded, and so on..)
    – Torbjørn
    Commented Jan 12, 2016 at 14:28
  • I agree with @Torbjørn - not sure the const statements add anything here, but still much better than the original...
    – Robbie Dee
    Commented Jan 12, 2016 at 14:38
  • @RobbieDee - I agree this is much better than the original one. But I think I would go with Torbjørn's solution. In fact, I probably would have thought of this if I would have done it myself, but when I look at it, the arguments of AddMessage call somewhat do not provide as to what is the information that is being passed. Though, it makes the code much cleaner. Commented Jan 12, 2016 at 14:43

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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