1
\$\begingroup\$

As a follow-up to this question that was an attempt to add basic translation capabilities in an old c++98 program for the sake of the community, I'm posting here the first iteration of my code after applying some of the points that emerged from the previous question:

#include <string>
#include <iostream>

// Not nested in class definition to avoid class namespace in code
enum
   {
    TR_RELOAD=0,
    TR_SAVE,
    TR_SIZ
   };

class PoorMansTranslations
{
 private:
    std::string i_tr[TR_SIZ]; // Actual translation strings

    // Here to be near the enum, consistency is by hand!
    typedef char tr_chars_t[TR_SIZ][16]; // Translation string tables with static linkage
    static const tr_chars_t& resolve_lang(const std::string& lang) throw()
       {
        static const tr_chars_t tr_en =
           {
            "Reload", // TR_RELOAD
            "Save"    // TR_SAVE
           };

        static const tr_chars_t tr_it =
           {
            "Ricarica", // TR_RELOAD
            "Salva"     // TR_SAVE
           };

        static const tr_chars_t tr_es =
           {
            "Recargar", // TR_RELOAD
            "Salvar",   // TR_SAVE
           };

        static const tr_chars_t tr_fr =
           {
            "Recharger",  // TR_RELOAD
            "Enregistrer" // TR_SAVE
           };

             if(lang=="en") return tr_en;
        else if(lang=="it") return tr_it;
        else if(lang=="fr") return tr_fr;
        else if(lang=="es") return tr_es;
        return tr_en; // Fallback lang
       }

 public:
    const std::string& operator[](const std::size_t idx) const throw() { return i_tr[idx]; }

    void select_lang(const std::string& lang) throw()
       {
        const tr_chars_t& tr_arr = resolve_lang(lang);
        // Constructing the strings on demand
        for(int i=0; i<TR_SIZ; ++i) i_tr[i] = std::string(tr_arr[i]);
       }
} tr;


int main()
{
    std::string lang = "it"; // note: unknown at compile time

    tr.select_lang(lang);
    std::cout << tr[TR_RELOAD] << ", " << tr[TR_SAVE] << '\n';

    lang = "bad";
    tr.select_lang(lang);
    std::cout << tr[TR_RELOAD] << ", " << tr[TR_SAVE] << '\n';
}
```
\$\endgroup\$
1
\$\begingroup\$

Avoid hardcoding translation strings to 16 bytes

There's no need to set the size of the strings to 16 bytes, just store them as pointers to const char instead of arrays of char:

typedef const char *tr_chars_t[TR_SIZ];

Store all languages in an array

For each language you have a separate variable holding the translations strings, and you have a long if-then-chain to find the right variable given the languages name. You can make the code more generic by storing all languages in an array, like so:

static const tr_chars_t &resolve_lang(const std::string &lang) throw()
{
    static const struct {
        const char *lang;
        const tr_chars_t translations;
    } languages[] = {
        {
            "en", {
                "Reload", // TR_RELOAD
                "Save"    // TR_SAVE
            }
        },
        {
            "it", {
                "Ricarica", // TR_RELOAD
                "Salva"     // TR_SAVE
            }
        },
        ...
    };

    static const std::size_t n_languages = sizeof languages / sizeof * languages;

    for (std::size_t i = 0; i < n_languages; ++i) {
        if (lang == languages[i].lang) {
            return languages[i].translations;
        }
    }

    return languages[0].translations; // Fallback lang
}

This way, when adding a language, you only have to add an entry to the array languages[], and not change any other code.

You could even improve it a bit by ensuring the languages are ordered alphabetically, so you can do a binary search.

Consider using STL algorithms

Copying the C strings in to the array of std::strings can be done with std::copy():

void select_lang(const std::string &lang) throw()
{
    const tr_chars_t &tr_arr = resolve_lang(lang);
    std::copy(tr_arr, tr_arr + TR_SIZ, i_tr);
}

You could also use std::find() or std::lower_bound() to replace the for-loop in my version of resolve_lang(), but it's a bit awkward in C++98 since you can't use a lambda for the comparator.

Consider storing the translation strings in std::strings

Instead of storing the translation strings in arrays of regular C strings, you can also consider storing them in arrays of std::strings. The drawback is that they will all be initialized once at runtime, but then you can just return a reference to that array, instead of copying the C strings into i_tr. Which method is best depends on how many languages you want to support versus how often you switch languages in your code multiplied by the number of strings in a language.

\$\endgroup\$
4
  • \$\begingroup\$ A compromise is to make an array of string_view which avoids copying the bytes but give the performance of knowing the actual length without scanning for the terminator. As I noted in my earlier answer, with C++98 you can include such a class as part of your project even if it's not in your copy of the standard library that's old enough to drink. \$\endgroup\$
    – JDługosz
    Sep 1 at 13:42
  • \$\begingroup\$ I'll definitely apply the first points (not sure where post the code update), regarding storing all in std::string I think the advantage depends on how frequent is the switching between langs, in my use case it occurs once at start. \$\endgroup\$
    – MatG
    Sep 1 at 14:07
  • \$\begingroup\$ Here's the next version \$\endgroup\$
    – MatG
    Sep 3 at 21:20
  • \$\begingroup\$ If you want the next version reviewed, you're welcome to post it as a new question on CodeReview! \$\endgroup\$
    – G. Sliepen
    Sep 3 at 21:43

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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