4
\$\begingroup\$

We had a long HTML string with information about the makers of our software. However, for i18n this was unwieldy. So I started to refactor, so only each person's description is translateable, not all HTML tags, nor contact info. I am beginner, so probably not used the best data structure, etc.

This was the code before:

    QString aboutMudletBody(
        tr("<p align=\"center\"><big><b>Original author: <span style=\"color:#bc8942;\">Heiko Köhn</span></b> (<b><span style=\"color:#0000ff;\">KoehnHeiko@googlemail.com</span></b>)</big></p>\n"
           "<p align=\"center\"><big><b>Credits:</b></big></p>"
           "<p><span style=\"color:#bc8942;\"><big><b>Vadim Peretokin</b></big></span> (<span style=\"color:#7289DA;\">Vadi#3695</span> <span style=\"color:#40b040;\">vadi2</span> <span style=\"color:#0000ff;\">vadim.peretokin@mudlet.org</span>) GUI design and initial feature planning. He is responsible for the project homepage and the user manual. Maintainer of the Windows, macOS, Ubuntu and generic Linux installers. Maintains the Mudlet wiki, Lua API, and handles project management, public relations &amp; user help. With the project from the very beginning and is an official spokesman of the project. Since the retirement of Heiko, he has become the head of the Mudlet project.</p>\n"
           "<p><span style=\"color:#bc8942;\"><big><b>Stephen Lyons</b></big></span> (<span style=\"color:#7289DA;\">SlySven#2703</span> <span style=\"color:#40b040;\">SlySven</span> <span style=\"color:#0000ff;\">slysven@virginmedia.com</span>) after joining in 2013, has been poking various bits of the C++ code and GUI with a pointy stick; subsequently trying to patch over some of the holes made/found.  Most recently he has been working on I18n and L10n for Mudlet 4.0.0 so if you are playing Mudlet in a language other than American English you will be seeing the results of him getting fed up with the spelling differences between what was being used and the British English his brain wanted to see.</p>\n"
           "<p><span style=\"color:#bc8942;\"><b>Ahmed Charles</b></span> (<span style=\"color:#40b040;\">ahmedcharles</span> <span style=\"color:#0000ff;\">acharles@outlook.com</span>) contributions to the Travis integration, CMake and Visual C++ build, a lot of code quality and memory management improvements.</p>\n"
           "<p><span style=\"color:#bc8942;\"><b>Chris Mitchell</b></span> (<span style=\"color:#40b040;\">Chris7</span> <span style=\"color:#0000ff;\">chrismudlet@gmail.com</span>) has developed a shared module system that allows script packages to be shared among profiles, a UI for viewing Lua variables, improvements in the mapper and all around.</p>\n"
           "<p>Others too, have make their mark on different aspects of the Mudlet project and if they have not been mentioned here it is by no means intentional! For past contributors you may see them mentioned in the <b><a href=\"https://launchpad.net/~mudlet-makers/+members#active\">Mudlet Makers</a></b> list (on our former bug-tracking site), or for on-going contributors they may well be included in the <b><a href=\"https://github.com/Mudlet/Mudlet/graphs/contributors\">Contributors</a></b> list on GitHub.</p>\n"
           "<br>\n"
           "<p>Many icons are taken from the <span style=\"color:#bc8942;\"><b><u>KDE4 oxygen icon theme</u></b></span> at <a href=\"https://web.archive.org/web/20130921230632/http://www.oxygen-icons.org/\">www.oxygen-icons.org <sup>{wayback machine archive}</sup></a> or <a href=\"http://www.kde.org\">www.kde.org</a>.  Most of the rest are from Thorsten Wilms, or from Stephen Lyons combining bits of Thorsten's work with the other sources.</p>\n"));

I have omitted some names, just kept enough to show small example.

This is the code now:

   QVector<QStringList> aboutBigMakers; // [name, discord, github, email, description (HTML escaped)]
    aboutBigMakers.append({QStringLiteral("Heiko Köhn"), 
                           QString(),
                           QString(),
                           QStringLiteral("KoehnHeiko@googlemail.com"),
                           tr("Original author.",
                              "about:Heiko")});
    aboutBigMakers.append({QStringLiteral("Vadim Peretokin"),
                           QStringLiteral("Vadi#3695"),
                           QStringLiteral("vadi2"),
                           QStringLiteral("vadim.peretokin@mudlet.org"),
                           tr("GUI design and initial feature planning. He is responsible for the project homepage and the user manual. "
                              "Maintainer of the Windows, macOS, Ubuntu and generic Linux installers. "
                              "Maintains the Mudlet wiki, Lua API, and handles project management, public relations &amp; user help. "
                              "With the project from the very beginning and is an official spokesman of the project. "
                              "Since the retirement of Heiko, he has become the head of the Mudlet project.",
                              "about:Vadi")});
    aboutBigMakers.append({QStringLiteral("Stephen Lyons"),
                           QStringLiteral("SlySven#2703"),
                           QStringLiteral("SlySven"),
                           QStringLiteral("slysven@virginmedia.com"),
                           tr("After joining in 2013, he has been poking various bits of the C++ code and GUI with a pointy stick; "
                              "subsequently trying to patch over some of the holes made/found. "
                              "Most recently he has been working on I18n and L10n for Mudlet 4.0.0 so if you are playing Mudlet in a language "
                              "other than American English you will be seeing the results of him getting fed up with the spelling differences "
                              "between what was being used and the British English his brain wanted to see.",
                              "about:SlySven")});

    QVector<QStringList> aboutMoreMakers;
    aboutMoreMakers.append({QStringLiteral("Ahmed Charles"),
                            QString(),
                            QStringLiteral("ahmedcharles"),
                            QStringLiteral("acharles@outlook.com"),
                            tr("Contributions to the Travis integration, CMake and Visual C++ build, "
                               "a lot of code quality and memory management improvements.",
                               "about:ahmedcharles")});
    aboutMoreMakers.append({QStringLiteral("Chris Mitchell"),
                            QString(),
                            QStringLiteral("Chris7"),
                            QStringLiteral("chrismudlet@gmail.com"),
                            tr("Developed a shared module system that allows script packages to be shared among profiles, a UI for viewing Lua variables, improvements in the mapper and all around.",
                               "about:Chris7")});

    QString aboutMudletBody("<p align=\"center\"><big><b>Credits:</b></big></p>");
    QVectorIterator<QStringList> iterateBig(aboutBigMakers);
    while (iterateBig.hasNext()) { aboutMudletBody.append(createMakerHTML(iterateBig.next(), true)); }
    QVectorIterator<QStringList> iterateMore(aboutMoreMakers);
    while (iterateMore.hasNext()) { aboutMudletBody.append(createMakerHTML(iterateMore.next(), false)); }

    aboutMudletBody.append(
        tr("<p>Others too, have make their mark on different aspects of the Mudlet project and if they have not been mentioned here it is by no means intentional! For past contributors you may see them mentioned in the <b><a href=\"https://launchpad.net/~mudlet-makers/+members#active\">Mudlet Makers</a></b> list (on our former bug-tracking site), or for on-going contributors they may well be included in the <b><a href=\"https://github.com/Mudlet/Mudlet/graphs/contributors\">Contributors</a></b> list on GitHub.</p>\n"
           "<br>\n"
           "<p>Many icons are taken from the <span style=\"color:#bc8942;\"><b><u>KDE4 oxygen icon theme</u></b></span> at <a href=\"https://web.archive.org/web/20130921230632/http://www.oxygen-icons.org/\">www.oxygen-icons.org <sup>{wayback machine archive}</sup></a> or <a href=\"http://www.kde.org\">www.kde.org</a>.  Most of the rest are from Thorsten Wilms, or from Stephen Lyons combining bits of Thorsten's work with the other sources.</p>\n"));

It is much longer, but more readable, and also only the important part is in tr() for translators to review.

For combination with the actual HTML tags I made another function:

QString dlgAboutDialog::createMakerHTML(const QStringList aboutMaker, const bool big) const
{
    QString makerHTML;
    auto realname = aboutMaker.at(0);
    auto discord = aboutMaker.at(1);
    auto github = aboutMaker.at(2);
    auto email = aboutMaker.at(3);
    auto description = aboutMaker.at(4);    
    makerHTML.append(QStringLiteral("<p><span style=\"color:#bc8942;\">"));
    if (big) {makerHTML.append(QStringLiteral("<big>"));}
    makerHTML.append(QStringLiteral("<b>"));
    makerHTML.append(realname);
    makerHTML.append(QStringLiteral("</b>"));
    if (big) {makerHTML.append(QStringLiteral("</big>"));}
    makerHTML.append(QStringLiteral("</span> ("));
    if (!discord.isEmpty()) {
        makerHTML.append(QStringLiteral("<span style=\"color:#7289DA;\">"));
        makerHTML.append(discord);
        makerHTML.append(QStringLiteral("</span> "));
    }
    if (!github.isEmpty()) {
        makerHTML.append(QStringLiteral("<span style=\"color:#40b040;\">"));
        makerHTML.append(github);
        makerHTML.append(QStringLiteral("</span> "));
    }
    if (!email.isEmpty()) {
        makerHTML.append(QStringLiteral("<span style=\"color:#0000ff;\">"));
        makerHTML.append(email);
        makerHTML.append(QStringLiteral("</span> "));
    }
    makerHTML.append(QStringLiteral(") "));
    makerHTML.append(description);
    makerHTML.append(QStringLiteral("</p>\n"));
    return makerHTML;
}

It's a bit tricky, because not all makers have all contact info filled out. Some have none at all, some have only one listed, others two or all three.

Maybe we need to think about rephrasing that line as well. For now, my goal was not to change the output in the software at all. (I did so for the first maker after all, just to keep the overall layout.) The goal was to reduce and sanitize the output for translators, and that goal is accomplished.

Looking forward to your feedback on how to improve! :)

edit:

I have got two hints:

  1. All the appending of strings could be not very smart. Use a format string instead and fill in the spaces with .arg() function.
  2. Define my own data structure other than the anonymous list of strings. Maybe include the bool big in there as well.

edit:

I got rid of most appending in the createMakerHTML helper function, which now looks like this:

QString dlgAboutDialog::createMakerHTML(const QStringList aboutMaker, const bool big) const
{
    auto realname = aboutMaker.at(0);
    auto discord = aboutMaker.at(1);
    auto github = aboutMaker.at(2);
    auto email = aboutMaker.at(3);
    auto description = aboutMaker.at(4);

    QString coloredText("<span style=\"color:#%1;\">%2</span>");
    QStringList contactDetails;
    if (!discord.isEmpty()) {contactDetails.append(coloredText.arg("7289DA", discord));}
    if (!github.isEmpty()) {contactDetails.append(coloredText.arg("40b040", github));}
    if (!email.isEmpty()) {contactDetails.append(coloredText.arg("0000ff", email));}

    return QStringLiteral("<p>%1%2 %3</p>\n") // name (big?), contacts (if any?), description
        .arg(coloredText.arg("bc8942", QStringLiteral("<b>%1</b>")
             .arg((big) ? QStringLiteral("<big>%1</big>").arg(realname) : realname)),
             (contactDetails.isEmpty()) ? QString() :
                 QStringLiteral(" (%1)").arg(contactDetails.join(QChar::Space)),
             description);
}

Now that .arg() cascade is much shorter but very dense, feels a bit hard to grasp.

edit: Now also replaced the QStringList with a struct I defined. Let me know if you want to see the code.

\$\endgroup\$
  • \$\begingroup\$ Have you tried using the CTML Html document library, made for c++? \$\endgroup\$ – tinstaafl Aug 4 at 2:28
  • \$\begingroup\$ No, I did not even know that library. Thanks for the hint. Will look into it. \$\endgroup\$ – Lecker Kebap Aug 4 at 22:31
1
\$\begingroup\$

Avoid explicitly constructing QString and QStringLiteral objects

In many cases, it's totally unnecessary to explicitly write QString("...") or QStringLiteral("..."). Constant string literals will be implicitly converted to QString in many cases. Also, while QStringLiteral might avoid a copy in some cases, it looks like premature optimization to me. So for example, just write:

aboutBigMakers.append({"Heiko Köhn",
                       "",
                       "",
                       "KoehnHeiko@googlemail.com",
                       tr("Original author.", "about:Heiko")});

Use a struct for structured data

You are using a QStringList to hold the name, discord, github, email and description for an author. Now you have the problem that you have to remember the correct order of all the items in a QStringList, and it is easy to accidentily forget to add all the items to the list (especially empty ones). Instead, create a struct to hold these 5 items explicitly:

struct authorInformation {
    QString realname;
    QString discord;
    QString github;
    QString email;
    QString description;
};

Then you can construct the vector of author information like this:

QVector<authorInformation> aboutBigMakers;
aboutBigMakers.append({"Heiko Köhn",
                       "",
                       "",
                       "KoehnHeiko@googlemail.com",
                       tr("Original author.", "about:Heiko")});

Nothing much changed here, but in dlgAboutDialog::createMakerHTML() you no longer need to remember the order of the elements in the list, and can just write:

QString dlgAboutDialog::createMakerHTML(const authorInformation &aboutMaker, const bool big) const
{
    QString coloredText("<span style=\"color:#%1;\">%2</span>");
    QStringList contactDetails;
    if (!aboutMaker.discord.isEmpty()) {
        contactDetails.append(coloredText.arg("7289DA", aboutMaker.discord));}
    }
    ...
}

While you are at it, you can also add a bool big to struct authorInformation, so you don't have to pass that in as a separate function parameter, and you don't have to have one list for big and one for small authors anymore.

Consider using raw string literals (C++11)

You have a lot of strings containing HTML code that needs to escape double quote characters (\"). You can avoid this by using raw string literals introduced in C++11. For example, you can do this:

makerHTML.append(R"(<p><span style="color:#bc8942;">)");

Use a template library

You are actually implementing a template for displaying author information. It would be much nicer if you could just write the template as one big string, and have the various items like name, email address and so on filled in in the right spots. You could use string formatting like you do with coloredText, but a real template language is more appropriate here. I can recommend mstch, which is a C++ implementation of the {{mustache}} template language. Then you can write the whole HTML structure as one big string, and you avoid concattenating lots of tiny strings, as well as avoiding an .arg() cascade.

\$\endgroup\$

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.