Moving your strings to a separate file is a good idea! It keeps them together and will let you easily change them if required. Your question doesn't say you want to be able to translate them, but centralizing will help that to.
But, code like:
Messages.Add(sMsgText1 + '' + sName + '' + sMsgText2 + '' + sFullPath)
is not better than code like:
Messages.Add('The voucher was NOT sent to ' + sName+
' because the application is in TEST MODE.');
You've turned a messy but readable function call into a messy and un-readable function call. With the old code (the second snippet just above), you can read the code and see roughly what the message is going to say, because a lot of it is there in text. With the new code, you can't.
Second, the reason for moving the strings to to keep related items together and make it easier to change them. What if you want to change the above message so that instead of saying "The file 'foo' in path 'bar'..." it is phrased "The file bar\foo is..."? You can't: the way the messages are built is still fixed and scattered throughout your code. If you want to change several messages to be formatted the same way, you will need to change lots of individual places.
This will be even more of a problem if your goal is to translate your messages, since often translation requires rephrasing a message not just translating the components. (You need to change the order of subitems included in your messages, for example - you can't just assume each language is a phrase-for-phrase in order substitution.)
Refactor one step further
I'd suggest instead a more aggressive refactoring of your message code. You're definitely on the right track when you suggest moving your messages to a separate file. But don't just move the strings: move the functions as well. Instead of a large number of Messages.Add('...') scattered through your code, find the common subset of messages you create. Many will be very similar. Create a family of functions you can call, so that all similar messages are implemented with a single function, and if you need to change the phrasing for them, you can do it in a single spot.
For example, instead of:
Messages.Add('The file ' + sFile + ' in ' + sPath + ' was not found.');
... and elsewhere:
Messages.Add('The file ' + sFileName + ' in ' + sURL + ' was not found.');
have a single function:
Messages.ItemNotFound(sFile, sPath);
...
Messages.ItemNotFound(sFileName, sURL);
You get:
- Centralized message strings
- Centralized message functions
- Less code duplication
- Cleaner code (no assembling of strings in a function call, just parameters)
- Easier to translate - provide an alternate implementation of the functions (don't forget that just translating the substrings may not be enough, you often need to be able to alter the phrasing substantially.)
- Clear descriptions of what the message is in the function name, such as
ItemNotFount(item, path), which leads to
- Clearer code when you're reading it
Sounds good to me :)
constdeclarations. - David Heffernan