12
votes

This may not be the correct place for this question, if not feel free to move it. I tagged as Delphi/Pascal because it's what I am working in atm, but this could apply to all programming I guess.

Anyway I am doing some code cleanup and thinking of moving all the strings in my program to a separate single .pas file. Are there any pros and cons to doing this? Is it even worth doing?

To clarify: I mean that I will be creating a separate file, Strings.pas in it I will make all my text string variables.

Ex

Current Code

 Messages.Add('The voucher was NOT sent to ' + sName+
                          ' because the application is in TEST MODE.');
 Messages.Add('Voucher Saved to ' + sFullPath);
 Messages.Add('----------------------------------------------------------');

New Code would be something like:

Messages.Add(sMsgText1 + '' + sName + '' + sMsgText2 + '' + sFullPath)

The Strings.pas file would hold all the string data. Hope that makes better sense

6
Why would you do such thing?You can of course place the strings in some other .pas file,then parse the .pas file in your file that contains code and use that strings,though in that case,I'll give the strings file extension .txt. - user744186
You can do that by loading the file into a string then parsing the string.I don't know pascal,so I can't give you an example. - user744186
@Aleksandar OP is thinking of writing a .pas file that contains a long list of const declarations. - David Heffernan
I don't know about pascal.But in c,you can declare a header file,containing only constants(defines,typedefs and const vars) - user744186
@Ken: I think it is a rather universal problem, that has a similar solution in C, C#, etc. as it has in Pascal. But one wonders a little indeed. - Rudy Velthuis

6 Answers

12
votes

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 :)

7
votes

I think it makes a lot of sense to move all string constants to a single unit. It makes changing the texts a lot easier, especially if you want to translate to other (human) languages.

But instead of strings, why don't you do what I usually do, i.e. use resourcestring. That way, your strings can be changed by someone else with a resource editor, without recompilation.

unit Strings;

resourcestring
  strMsgText1 = 'The voucher was NOT sent to ';

etc...

But such a string is probably better done as:

resourcestring
  strVoucherNotSent = 
    'The voucher was NOT sent to %s because the application is in TEST MODE.';
  strVoucherForNHasValueOf =
    'The voucher for %s has a value of $%.2f'; 

The advantage of that is that in some languages, the placement and order of such substitutions is different. That way, a translator can place the arguments wherever it is necessary. Of course the application must then use Format() to handle the string:

Messages.Add(Format(strVoucherNotSent, [sName]));
Messages.Add(Format(strVoucherSavedTo, [sFullPath]));
Messages.Add(Format(strVoucherForNHasValueOf, [sName, dblValue]));
5
votes

If you are wanting to translate the UI into different languages then you may benefit from having all your text in a single file, or perhaps a number of files dedicated to declaring string constants.

However, if you do this change without such a strong motivation, then you may just make your code hard to read.

Generally you have to ask what the benefits of such a major refactoring are, and if they are not self-evident, then you may well be changing things just for the sake of change.

3
votes

If you want to translate your app, consider using Gnu GetText for delphi also known as dxGetText. This is better than putting your strings into a separate .pas file because it allows you to enable translation without any special tools, any recompilation, by even end users.

0
votes

Well, the consensus here seems to tend towards the pro side, and I agree with that completely. Wrong overuse of string literals could lead to your source getting stringly typed.

One benefit I am still missing is reusability of strings. Although that is not a direct benefit from the shipping to another unit, it ís from moving the string literals to constants.

But a rather significant drawback is the required time to create this separate source file. You might consider to implement this good programming practice in the next project. It entirely depends on whether you have sufficient time (e.g. hobby project) or a deadline, on having a next project or not (e.g. student), or on just wanting to do some practice. Like David H answers and comments, as with all decisions you make, you have to weight the benefits.

Seen apart from all kinds of fancy refactoring tools that could provide in some automatic assistance, realize that moving the string literals to another unit alone does not get the job done. Like Rudy and David M already answered, you partly have to rewrite your source. Also, finding readable, short and applicable constant names dóes take time. As many comments already stated that having control over spelling an consistency is important, I like to think that same argument goes for the replacing constants itself as well.

And about the translation answers, whether being applicable to OP or not, moving all your source strings to a separate unit is only part of a translation solution. You also have to take care for designer strings (i.e. captions), and GUI compatibility: a longer translation still has to fit in your label.

If you have the luxury or the need to: go for it. But I would take this to the next project.

0
votes

I did group all literals with resourcestrings in an older version of our framework. I came back from that since in frameworks you might not use all strings in a framework (e.g. because some units or unit groups are not used, but scanning the common dirs will show up all strings in your translation tool).

Now I distribute them again over units. I originally started grouping them to avoid duplicates, but in retrospect that was the lesser problem

(*) using dxgettext on the "common" dirs.