2
\$\begingroup\$

I have a string which contains JSON-like object flattened (stringify()ed).

I need to JSON-ify it back. It's a custom format, so I wrote function to JSON-ify the string:

#include <string>
#include <unordered_set>
#include <iostream>

const std::unordered_set<std::string> reservedWords{ "coords", "comment" };

std::string jsonify(const std::string &tclString)
{
    std::string result;

    int level = 0;
    const int tabWidth = 4;

    bool reservedWord = false;
    const auto length = tclString.length();
    std::string prevWord;
    for (auto idx = 0; idx < length; ++idx)
    {
        auto ch = tclString[idx];
        if (ch == ' ')
            result += ' ';
        else if (ch == '{')
        {
            std::string word;
            while (++idx < length)
            {
                ch = tclString[idx];
                if (ch == ' ' || ch == '}')
                    break;
                word += ch;
            }

            result += '{';
            if (word == "}")
            {
                if (reservedWord)
                    reservedWord = false;
            }
            else if (!reservedWord)
            {
                ++level;
                result += '\n';
                result += std::string(level * tabWidth, ' ');
                reservedWord = reservedWords.contains(word);
            }

            --idx;
            result += word;
        }
        else if (ch == '}')
        {
            if (reservedWord)
            {
                result += '}';
                reservedWord = false;
            }
            else
            {
                --level;
                result += '\n';
                result += std::string(level * tabWidth, ' ');
                result += '}';
                result += '\n';
            }
        }
        else
        {
            std::string word;
            while (idx < length)
            {
                if (ch == '}' || ch == ' ' || ch == '{')
                    break;
                word += ch;
                ++idx;
                ch = tclString[idx];
            }
            --idx;
            result += word;
            if (!reservedWord)
                reservedWord = reservedWords.contains(word);
        }
    }

    return result;
}

int main()
{
    std::cout << jsonify("{data {a {obj {coords {10 10} comment {} radius 260 type circle}}}}") << '\n';
    std::cout << jsonify("{data {b {obj {coords {-95 -85 -70 -85 -75 -95} comment {abc} type polygon}}}}") << '\n';
    std::cout << jsonify("{data {c {obj {coords {-55 -65 -70 -65 -25 -64} comment {abc def} type polygon}}}}") << '\n';
}

Please review the code and let me know how I can make it better?

Thanks in advance.

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$
  1. All the functionality is written in one fat function, which prevents modularization of code. Try to break up it into subfunctions; you could separate:

        std::string word;
        while (++idx < length)
        {
            ch = tclString[idx];
            if (ch == ' ' || ch == '}')
                break;
            word += ch;
        }
    

    This is simply trying to tokenize the input into words. You could do that in another function.

  2. I see repetitions of similar logic for tokenizing in the else part, albeit with minor modification; you could try and club it into one function and pass separators.

  3. I think the case is more suited for a while loop, then you don't have to spray around the:

    --idx
    
  4. The variable

    std::string prevWord
    

    is not used at all.

  5. Overall, the design is not flexible enough and does not incorporate OCP. If you had to accommodate any change you would be changing the function. Consider using something like a fluent builder pattern here. Depending upon how complex it is you can think of using javax.json.

\$\endgroup\$
2
  • \$\begingroup\$ Can you expand the acronym OCP and perhaps provide a link to its definition? Also a link to a description of the "fluent builder pattern" would help. \$\endgroup\$
    – G. Sliepen
    Jan 27 at 22:59
  • \$\begingroup\$ @Suryasis Paul thanks for the feedback. I have updated the code in new post: codereview.stackexchange.com/questions/273552/…. can you please take a look again? \$\endgroup\$ Jan 30 at 18:01

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.