Prettify JSON class

It's not much, but I tried employing some of the things I learned yet never really got to use, since that kind of code isn't really needed where I work (for the most part). I tried making it as much C++11 as I could. I would like to know if I messed up somewhere, maybe something could have been done better, or maybe you got some tips to improve readability?

```
#include
#include
#include

class JSONPretify : public std::string{

public:

JSONPretify(std::string j){
this->assign(j);
pretify();
};
JSONPretify(std::string j, bool colon_space){
this->assign(j);
pretify();
if(colon_space)
insertColonSpaces();
};
private:
void pretify(){
std::regex var = std::regex(R"((\".+?\".*?(?=\{|\[|\,|\]|\}))|(\d+?))");
long it = 0;
int depth = 0;
while(it size() && it != -1){
regex_pos pos_tab = findRegexFirstPosition(it, var);
long pos_comma = this->find(",", it);
long pos_obj_start = this->find("{", it);
long pos_obj_end = this->find("}", it);
long pos_array_start = this->find("[", it);
long pos_array_end = this->find("]", it);
long old_it = it;

unsigned long work_with = find_lowest(std::vector{pos_tab.pos, pos_comma, pos_obj_start, pos_obj_end,pos_array_start,pos_array_end});

switch(work_with){
case(TAB):{
std::string insert = generateSpaces(depth);
this->insert(pos_tab.pos, insert);

it = pos_tab.pos+insert.size()+pos_tab.length;
break;
}
case(COMMA):{
std::string insert = "\n";
this->insert(pos_comma+1, insert);

it = pos_comma+1;
break;
}
case(OBJ_START):{
std::string insert = "\n";
this

Thanks for editing the question. I have few ideas how you might want to polish your code.

std::string base class

It caught my eye that JSONPretify is derived from std::string. Generaly speaking STL containers are not meant for such use case. Specificaly it is a good idea if you are deriving from a base class to check that it has virtual destructor. std::string does not have virtual destructor.

See e. g.
https://www.securecoding.cert.org/confluence/display/cplusplus/OOP52-CPP.+Do+not+delete+a+polymorphic+object+without+a+virtual+destructor

interface

I would go even further and suggest that as you don't need to model any state, keep any data or invariant simple function might be better interface.

std::string pretify(const std::string& j, bool colon_space = false);

separation of interface and implementation

In order to be able to hide all hairy details from users of your code you might separate it into interface and implementation. The most common form is iterface only header file (e. g. prettify.hpp) and implementation source file (e. g. prettify.cpp). You then might leave all definitions and implementation details for prettify.cpp. To separate it from the rest of your code (even that you are only linking to) you might use either anonymous namespaces or internal linkage functions (surprisingly this is other meaning of keyword static).

• http://en.cppreference.com/w/cpp/language/namespace

• http://en.cppreference.com/w/cpp/language/storage_duration

find_lowest

I would try to avoid implementation of this algorithm and use std::string::find_first_of() and/or std::min_element().

If you decide to stick with it then you still might simplify it by not making it a template as there is just single call to it. You also probably don't want to copy the argument vector so reference might be more appropriate:

unsigned long find_lowest(const std::vector& outof){

work_with

I would recommend using scoped enum (great C++11 extension) and distinquishing between such enum and integers.

See http://en.cppreference.com/w/cpp/language/enum [Scoped enumerations]

All it takes is new positions definition:

enum class positions{

and change to values usage:

case(positions::TAB):{

I am perplexed by this

positions work_with = find_lowest(std::vector{pos_tab.pos, pos_comma, pos_obj_start, pos_obj_end,pos_array_start,pos_array_end});

because you are assigning position to work_with but checking content in switch. Is it correct?

variables

This is kind of subjective opinion but omitting some helper variables might increase readability.

case(COMMA):{
                std::string insert = "\n";
                this->insert(pos_comma+1, insert);

                it = pos_comma+1;
                break;
            }

shortened to

case(COMMA):{
                this->insert(pos_comma+1, "\n");

                it = pos_comma+1;
                break;
            }

For those variables that you create and don't intend to change I would definitely use const to let know the compiler about your intention and let it actually check that you don't accidentally violate that.

const std::regex var = std::regex(R"((\".+?\".*?(?=\{|\[|\,|\]|\}))|(\d+?))");

const regex_pos pos_tab = findRegexFirstPosition(it, var);

const std::string insert = generateSpaces(depth);

insertColonSpaces

Basically you are replacing one string with another.

This question might give some hints (e. g. boost::algorithm::replace_all_copy or using std::regex).
https://stackoverflow.com/questions/5343190/how-do-i-replace-all-instances-of-a-string-with-another-string

generateSpaces

Unless I have overlooked something it is the same as

return std::string(l * 4, ' ');

Check std::string "fill" constructor here:
http://www.cplusplus.com/reference/string/string/string/

for() { break; }

This loop

for (unsigned i=0; i<m.size(); ++i) {
        at = m.position(i);
        l = m[i].str().size();
        break;
    }

looks more like a simple condition

if ( m.size() > 0 ) {
        at = m.position(0);
        l = m[0].str().size();
    }

StackExchange Code Review Q#132790, answer score: 16