Inventorizing the universe

Question

I recently started re-factoring a project I hadn't touched in many months.

The original purpose was to be an inventory management system for a game I played, Ogame. In this game you play inside a universe filled with galaxies, which are filled with systems, which are filled with planets. Those planets have a coordinate consisting of three parts (galaxy, system, location in system). The usual sizes are 9 galaxies, 499 systems and 15 planets per system, which allows for 67365 planets.

I wrote a pseudo-CSV reader (the first two separators are : instead of ; because that's what's used in-game as well) which can sort planets based on coordinates or properties. The properties of relevance are 3 mining facilities and the Solar (Power) plant.

Note negative values shouldn't exist. Although I feel it's the user's responsibility not to input those, I'm not checking for it either. This is by design, but may be a bad practice.

The brief UI is also by design. I think it's intuitive enough (especially since I'm currently the only user), but if it's possible to achieve the same result with more elegance I'm all for it.

#CSV (Galaxy.txt):

1:2:12;20;18;15;0;7;6;4;
1:5:12;20;18;11;0;6;5;3;
1:9:4;10;7;6;0;1;0;0;
1:9:8;17;14;8;0;3;2;1;
1:9:9;18;16;7;0;4;3;2;
1:10:5;9;6;2;0;2;1;0;
1:10:12;2;2;2;0;1;0;0;
1:11:11;19;16;10;0;5;3;1;
1:11:12;23;20;16;0;7;6;5;
1:12:4;2;2;2;0;0;1;0;
1:14:5;4;2;1;0;1;0;0;
1:15:5;20;18;10;0;5;3;1;
1:16:5;18;16;9;0;5;4;4;
1:20:7;24;21;6;0;8;7;2;
1:20:8;22;20;14;0;9;7;5;
1:20:9;23;22;13;0;8;7;4;
1:21:6;11;12;11;0;6;4;3;
1:21:7;23;20;13;0;7;6;3;
1:22:7;14;13;9;0;3;2;2;
1:25:6;19;18;12;0;7;6;3;
1:25:11;16;15;4;0;6;5;3;
1:27:12;12;11;8;0;6;4;2;
1:32:6;19;17;13;0;6;5;3;
1:38:7;22;21;12;0;8;7;6;
1:38:10;18;14;13;0;7;5;4;
1:39:5;18;15;10;0;6;5;4;
1:39:12;20;18;12;0;7;6;4;
1:40:6;9;8;7;0;3;2;2;
1:42:7;19;18;15;6;6;5;4;
1:43:11;19;19;14;5;7;7;7;
1:43:12;19;19;14;5;7;7;7;
1:45:4;20;18;16;0;8;7;7;
1:45:5;20;17;16;0;7;6;5;
1:45:9;15;15;12;0;5;5;4;
1:45:13;15;15;15;0;5;5;4;
1:46:12;11;11;3;0;2;2;1;
1:56:8;22;18;12;0;7;6;5;
1:59:6;23;21;21;0;8;7;6;
1:61:7;17;16;17;5;2;1;2;
1:62:4;14;13;13;0;4;4;2;
1:62:10;3;2;0;0;0;0;0;
1:63:5;10;8;4;0;4;3;2;
1:64:3;5;2;1;0;2;2;0;
1:64:4;21;15;12;0;8;6;3;
1:64:8;9;9;6;0;3;2;2;
1:65:5;14;9;4;0;5;3;1;
1:65:8;23;20;17;7;8;7;7;
1:65:9;23;21;16;7;8;7;7;
1:65:10;13;16;14;0;5;5;6;
1:65:11;24;22;16;7;9;7;7;
1:67:4;17;16;10;0;5;5;3;
1:67:5;17;16;9;0;5;5;2;
1:152:7;16;14;12;0;5;5;4;
1:153:6;23;22;17;0;8;7;9;
1:154:8;23;20;18;0;6;6;3;
1:292:8;24;19;11;0;7;6;3;
2:157:6;24;20;20;6;9;6;6;
2:157:7;23;20;20;6;8;6;6;
2:157:12;10;7;2;0;1;1;0;

• I think I got the Single Responsibility Principle right on most things, although there are still a couple of large functions.

• The naming isn't too bad, but could probably be better.

• There is much repetition in the sorting functions, which is bad.

• The maintainability-factor of this code is quite bad.

• I tried to keep an overview at the start of Main.cpp with available classes and functions. It feels obsolete since the maintainability versus profit doesn't seem worth it.

• I'm probably copying too much data. There are probably optimizations to be gained in how I pass variables around. I suspect I'm not using enough pointers.

#Main.cpp

#ifndef MAIN_CPP
#define MAIN_CPP

/*

Project :   Universe
By      :   Mast
Date    :   August '14

structUniverse
--------
sName                                           :   std::string
vPlanet                                         :   std::vector<Planet> vPlanet;

vsSanitizeData(std::vector<std::string>)        :   std::vector<std::string>
sort_x();                                       :   void
sort_y();                                       :   void
sort_z();                                       :   void
sort_Mlvl();                                    :   void
sort_Clvl();                                    :   void
sort_Dlvl();                                    :   void
sort_Plvl();                                    :   void
readCSV();                                      :   void
addPlanet(Planet);                              :   void
splitString(std::string);                       :   int

getUniverse();                                  :   void
printUniverse();                                :   void


structPlanet
--------
x                                               : int
y                                               : int
z                                               : int
Mlvl                                            : int
Clvl;                                           : int
Dlvl;                                           : int
Plvl;                                           : int
Mstor;                                          : int
Cstor;                                          : int
Dstor;                                          : int

getPlanet();                                    : int
printPlanet();                                  : void
getMlvl();                                      : int
getClvl();                                      : int
getDlvl();                                      : int

*/

#include "Universe.hpp"

struct Planet;
struct Universe;

int main()
{
    Universe Uno("Uno");
    Uno.readCSV();

    bool exit = false;
    while (!exit)
    {
        system("cls");
        std::cout << "Welcome to the Galaxy Management Unit (GMU)." << '\n'
            << '\n'
            << "0. Print." << '\n'
            << '\n'
            << "1. Sort X." << '\n'
            << "2. Sort Y." << '\n'
            << "3. Sort Z." << '\n'
            << '\n'
            << "4. Sort Mlvl." << '\n'
            << "5. Sort Clvl." << '\n'
            << "6. Sort Dlvl." << '\n'
            << "7. Sort Plvl." << '\n'
            << '\n'
            << "999. Exit." << '\n'
            << '\n'
            << "Choice: "
            ;
        int choice;
        std::cin >> choice;
        switch (choice)
        {
        case 0:
            Uno.printUniverse();
            std::cin.sync();
            std::cin.clear();
            std::cin.get();
            break;
        case 1:
            Uno.sort_x();
            break;
        case 2:
            Uno.sort_y();
            break;
        case 3:
            Uno.sort_z();
            break;
        case 4:
            Uno.sort_Mlvl();
            break;
        case 5:
            Uno.sort_Clvl();
            break;
        case 6:
            Uno.sort_Dlvl();
            break;
        case 7:
            Uno.sort_Plvl();
            break;
        case 999:
            exit = true;
            break;
        default:
            break;
        }

    }

}

#endif

#Universe.hpp

#ifndef UNIVERSE_HPP
#define UNIVERSE_HPP

/*

Project : Universe
By      : Mast
Date    : August '14

*/

#include <algorithm>
#include <iostream>
#include <string>
#include <fstream>
#include <vector>

struct Planet
{
    int x;
    int y;
    int z;
    int Mlvl;
    int Clvl;
    int Dlvl;
    int Plvl;
    int Mstor;
    int Cstor;
    int Dstor;

    Planet(int x, int y, int z, int Mlvl, int Clvl, int Dlvl, int Plvl, int Mstor, int Cstor, int Dstor) : x(x), y(y), z(z), Mlvl(Mlvl), Clvl(Clvl), Dlvl(Dlvl), Plvl(Plvl), Mstor(Mstor), Cstor(Cstor), Dstor(Dstor) {}

    ~Planet(){};

    int getPlanet();
    void printPlanet();
    int getMlvl();
    int getClvl();
    int getDlvl();
};

int Planet::getPlanet()
{
    return x, y, z, Mlvl, Clvl, Dlvl, Plvl, Mstor, Cstor, Dstor;
}

void Planet::printPlanet()
{
    std::cout << x << ':'
        << y << ':'
        << z << ' '
        << Mlvl << ' '
        << Clvl << ' '
        << Dlvl << ' '
        << Plvl << ' '
        << Mstor << ' '
        << Cstor << ' '
        << Dstor
        << '\n';
}

int Planet::getMlvl()
{
    return Mlvl;
}

int Planet::getClvl()
{
    return Clvl;
}

int Planet::getDlvl()
{
    return Dlvl;
}

struct Universe
{
    std::string sName;
    std::vector<Planet> vPlanet;

    Universe(std::string sName) : sName(sName) {}

    ~Universe(){};

    std::vector<std::string> vsSanitizeData(std::vector<std::string>);
    void sort_x();
    void sort_y();
    void sort_z();
    void sort_Mlvl();
    void sort_Clvl();
    void sort_Dlvl();
    void sort_Plvl();
    void readCSV();
    void addPlanet(Planet);
    int splitString(std::string);

    void getUniverse();
    void printUniverse();
};

struct compare_x
{
    inline bool operator() (const Planet& left, const Planet& right)
    {
        return (left.x < right.x);
    }
};

struct compare_y
{
    inline bool operator() (const Planet& left, const Planet& right)
    {
        return (left.y < right.y);
    }
};

struct compare_z
{
    inline bool operator() (const Planet& left, const Planet& right)
    {
        return (left.z < right.z);
    }
};

struct compare_Mlvl
{
    inline bool operator() (const Planet& left, const Planet& right)
    {
        return (left.Mlvl < right.Mlvl);
    }
};

struct compare_Clvl
{
    inline bool operator() (const Planet& left, const Planet& right)
    {
        return (left.Clvl < right.Clvl);
    }
};

struct compare_Dlvl
{
    inline bool operator() (const Planet& left, const Planet& right)
    {
        return (left.Dlvl < right.Dlvl);
    }
};

struct compare_Plvl
{
    inline bool operator() (const Planet& left, const Planet& right)
    {
        return (left.Plvl < right.Plvl);
    }
};

void Universe::sort_x()
{
    std::sort(vPlanet.begin(), vPlanet.end(), compare_x());
}

void Universe::sort_y()
{
    std::sort(vPlanet.begin(), vPlanet.end(), compare_y());
}

void Universe::sort_z()
{
    std::sort(vPlanet.begin(), vPlanet.end(), compare_z());
}

void Universe::sort_Mlvl()
{
    std::sort(vPlanet.begin(), vPlanet.end(), compare_Mlvl());
}

void Universe::sort_Clvl()
{
    std::sort(vPlanet.begin(), vPlanet.end(), compare_Clvl());
}

void Universe::sort_Dlvl()
{
    std::sort(vPlanet.begin(), vPlanet.end(), compare_Dlvl());
}

void Universe::sort_Plvl()
{
    std::sort(vPlanet.begin(), vPlanet.end(), compare_Plvl());
}

std::vector<std::string> Universe::vsSanitizeData(std::vector<std::string> Vec)
{
    std::vector<std::string>::iterator vI = Vec.begin();
    while (vI != Vec.end())
    {
        if (*vI == "")
        {
            vI = Vec.erase(vI);
        }
        else
        {
            vI++;
        }
    }

    return Vec;
};

void Universe::readCSV()
{
    std::ifstream input("Galaxy.txt", std::ios::in);
    std::string sRead;
    std::vector<std::string> sData;
    std::vector<std::string>::iterator itData;
    while (input.good())
    {
        getline(input, sRead);
        sData.push_back(sRead);
    }
    sData = vsSanitizeData(sData);
    for (itData = sData.begin(); itData != sData.end(); itData++)
    {
        splitString(*itData);
        std::cout << '\n';
    }

}

void Universe::addPlanet(Planet p)
{
    vPlanet.push_back(p);
}

void Universe::getUniverse()
{
    for (unsigned i = 0; i < vPlanet.size(); i++)
    {
        vPlanet[i].getPlanet();
    }
}

void Universe::printUniverse()
{
    std::cout << '\n';
    for (unsigned i = 0; i < vPlanet.size(); i++)
    {
        vPlanet[i].printPlanet();
    }
}

int Universe::splitString(std::string sSplitInp)
{
    char primaryLimiter = ';';
    char secondLimiter = ':';
    std::string sTemp;
    std::vector<int> vTemp;

    for (size_t p = 0, q = 0; p != sSplitInp.npos; p = q)
    {
        sTemp = sSplitInp.substr(p + (p != 0), (q = sSplitInp.find(primaryLimiter, p + 1)) - p - (p != 0));
        std::cout << sTemp << ' ';
        if (p == 0)
        {
            std::string sNew;
            size_t j = 0, k = 0;
            for (unsigned i = 0; i <= 2; i++)
            {
                sNew = sTemp.substr(j + (j != 0), (k = sTemp.find(secondLimiter, j + 1)) - j - (j != 0));
                vTemp.push_back(atoi(sNew.c_str()));
                j = k;
            }
        }
        else
        {
            vTemp.push_back(atoi(sTemp.c_str()));
        }
    }
    addPlanet(Planet(vTemp[0], vTemp[1], vTemp[2], vTemp[3], vTemp[4], vTemp[5], vTemp[6], vTemp[7], vTemp[8], vTemp[9])); // ugly

    return 0;
}

#endif

Answer 1

• You don't need an include guard in Main.cpp

  #ifndef MAIN_CPP
  #define MAIN_CPP
  

  Include guards are meant to avoid a header file from being included more than once by the compiler/preprocessor (#include works just like text copy-pasting, so yes, the preprocessor is that dumb).

• For a struct that has all of its members publicly accessible, having get* methods is silly. However, it should be noted that "getters" are normally const methods, to allow them to be called on a const instance and to enforce at compile-time that member class data can't be modified inside the methods. E.g.:

  void printPlanet() const;
  int getMlvl() const;
  

  Also note that printPlanet() is not a getter, but it also falls into the category of only reading from the member data.

• I'd suggest moving the class implementations from Universe.hpp into a Universe.cpp file. It will make it easier to navigate just the interface or just the implementation.

• That summary comment in Main.cpp is superfluous and repeats itself with the declarations in Universe.hpp. If you move the implementation parts to a CPP file, as mentioned above, it will be much easier to read the declarations, so you won't need any extra summary or documentation.

• If you are open to C++11, then you can replace all those little comparator structures with inline lambdas. This will keep the sorting predicate together with the sort call. Better "locality of reference" to the reader.

• You should omit empty destructors and let the compiler provide the defaults.

Answer 2

I have a few questions about your design.

Classes

What's the purpose of using classes if all of your data and methods are public? Why bother having getters and setters like getMlvl(), getClvl(), getDlvl()? Personally, I'd make all the instance variables private and leave the accessors because if you later expand this it will be difficult to track who's changing parts of an object otherwise. But if you are going to make them all public, don't bother having accessors. (Or at least, be consistent and make them for all members or none.)

What is Planet::getPlanet() supposed to do? It returns an int, but the return statement has 10 variables it appears to be returning. My compiler is confused by this. It seems like maybe you're trying to make a copy of it? Like it should be:

Planet Planet::getPlanet()
{
    return Planet(x,y,z,Mlvl,Clvl,Dlvl,Plvl,Mstor,Cstor,Dstor);
}

But if that's what you intended, then maybe what you really need is a copy constructor?

Overall the Planet class seem unnecessary. Just making it a struct with the only the ints and a single non-class function printPlanet(const Planet&) would work just as well.

Where are the galaxies and systems? Your Universe class contains a single vector of Planets. I thought there was a hierarchy of Universe contains Galaxies contain Systems contain Planets? But I only see that Universe contains Planets. That seems odd.

Naming

I don't particularly like Hungarian notation. But if you are going to use it, be consistent. None of your member variables in Planet use it at all. I'd expect them to be named iX, iY, iZ, etc. Especially since coordinates are usually floating point. Also in Universe, you have vector<Planet> vPlanet. I'd expect it to be vpPlanet, and I'd probably make it plural since it usually holds more than 1 (unless it's a lonely Universe).

Same with the parameter to vsSanitizeData(). It should be named vsData or maybe something more descriptive like vsToSanitize.

I also have no idea what Clvl, Dlvl, Mlvl, and Plvl mean. Nor do I understand what Mstor, Cstor, or Dstor are. Don't be afraid to use full names like miningLevel or powerLevel (or whatever they stand for).

Code

Instead of having 7 different sort routines, I'd just have 1 that takes the compare structure as its parameter or maybe an enum saying which sort you want to do.

splitString() has a return value, but it's always 0, and it's ignored anyway. Also, it doesn't just split a string. It constructs a new Planet and adds it to the Universe. That seems like a bigger deal than splitting the string, so I'd change the name to reflect that. Maybe void addPlanetFromString(const std::string& planetStr)? Making the string a const reference will keep you from copying the string.

Answer 3

Commenting on the CSV format and reader, I would advice to change the following format:

1:2:12;20;18;15;0;7;6;4;

to

"1:2:12";20;18;15;0;7;6;4;

As you can see I'm treating the planet coordinates as a string rather than how it's done now and this will eliminate the need to have both a : and a ; delimiter.

Answer 4

This is not a complete review, but rather an elaboration on a single point already made. Having multiple ways of sorting things is not a bad idea, but there are better ways of doing it. Here are three different ways to do that which, I think, are successively better.

Use a lambda

As @glampert already said, your sort routines would be better expressed as a lambda:

void Universe::sort_x() { 
    std::sort(vPlanet.begin(), vPlanet.end(), 
        [](const Planet &a, const Planet &b){ 
            return a.x < b.x; 
        }); 
}

However, it's a bit messy to have to write that out for each member variable of Planet, so that leads to the next way to do it:

Use a macro

#define sort_on(x) sort_##x() { std::sort(vPlanet.begin(), vPlanet.end(), [](const Planet &a, const Planet &b){ return a.x < b.x; }); }

void Universe::sort_on(x)
void Universe::sort_on(y)
void Universe::sort_on(z)
void Universe::sort_on(Mlvl)
void Universe::sort_on(Dlvl)
void Universe::sort_on(Plvl)
void Universe::sort_on(Clvl)

Now all of the member functions are defined but it's still somewhat readable. It's still a bit unfortunate, though because it means Universe has to have intimate knowledge of and access to internal members of Planet. This is generally a red flag telling you that there may be something wrong with the class design.

Implement comparisons in the Planet class

You could have the comparison implementations as static members of the Planet class.

static bool byX(const Planet &a, const Planet &b) { return a.x < b.x; };
// etc.

Then you could have an enum within Planet:

enum sortby { X, Y, Z, MLVL, CLVL, DLVL, PLVL };

Then have a function which takes this enum as an argument and returns the appropriate function pointer.

static bool (*sorter(sortby field))(const Planet &a, const Planet &b) {
    switch (field) {
        case Y:
            return byY;
            break;
        case Z:
            return byZ;
            break;
        // all of the other cases
        default:
            return byX;
    } 
}

Then instead of having multiple sort functions in Universe, you would just have one:

void Universe::sort(Planet::sortby field) {
    std::sort(vPlanet.begin(), vPlanet.end(), 
            Planet::sorter(field));
}

A few more thoughts

Rather than doing all of that complicated I/O I'd suggest using the standard stream extractors and inserters. When you do so, the code shrinks considerably:

Universe.hpp

#ifndef UNIVERSE_HPP
#define UNIVERSE_HPP

/*

Project : Universe
By      : Mast
Date    : August '14

*/

#include <algorithm>
#include <iostream>
#include <string>
#include <fstream>
#include <vector>

class Planet
{
public:
    Planet() : x(0), y(0), z(0), Mlvl(0), Clvl(0), Dlvl(0), Plvl(0), Mstor(0), Cstor(0), Dstor(0) {}
    Planet(int x, int y, int z, int Mlvl, int Clvl, int Dlvl, int Plvl, int Mstor, int Cstor, int Dstor) : x(x), y(y), z(z), Mlvl(Mlvl), Clvl(Clvl), Dlvl(Dlvl), Plvl(Plvl), Mstor(Mstor), Cstor(Cstor), Dstor(Dstor) {}

    int getMlvl() const { return Mlvl; }
    int getClvl() const { return Clvl; }
    int getDlvl() const { return Dlvl; }
    enum sortby { X, Y, Z, MLVL, CLVL, DLVL, PLVL };
    static bool (*sorter(sortby field))(const Planet &a, const Planet &b) {
        switch (field) {
            case Y:
                return byY;
                break;
            case Z:
                return byZ;
                break;
            case MLVL:
                return byMLVL;
                break;
            case CLVL:
                return byCLVL;
                break;
            case DLVL:
                return byDLVL;
                break;
            case PLVL:
                return byPLVL;
                break;
            default:
                return byX;
        } 
    }
    friend std::istream &operator>>(std::istream &in, Planet &planet) {
        Planet p;
        char ch[9];
        int i=0;
        in  >> p.x >> ch[i++];
        in  >> p.y >> ch[i++];
        in  >> p.z >> ch[i++];
        in  >> p.Mlvl >> ch[i++];
        in  >> p.Clvl >> ch[i++];
        in  >> p.Dlvl >> ch[i++];
        in  >> p.Plvl >> ch[i++];
        in  >> p.Mstor >> ch[i++];
        in  >> p.Cstor >> ch[i++];
        in  >> p.Dstor >> ch[i++];
        if (!in) 
            return in;
        for (i=0; i < 2; ++i)
            if (ch[i] != ':') 
                in.setstate(std::ios_base::failbit);
        for (  ; i < 9; ++i)
            if (ch[i] != ';')
                in.setstate(std::ios_base::failbit);
        if (in) 
            std::swap(p, planet);
        return in;
    }
    friend std::ostream &operator<<(std::ostream &out, const Planet &p)
    {
        return out 
            << p.x << ':'
            << p.y << ':'
            << p.z << ' '
            << p.Mlvl << ' '
            << p.Clvl << ' '
            << p.Dlvl << ' '
            << p.Plvl << ' '
            << p.Mstor << ' '
            << p.Cstor << ' '
            << p.Dstor
            << '\n';
    }
    static bool byX(const Planet &a, const Planet &b) { return a.x < b.x; };
    static bool byY(const Planet &a, const Planet &b) { return a.y < b.y; };
    static bool byZ(const Planet &a, const Planet &b) { return a.z < b.z; };
    static bool byMLVL(const Planet &a, const Planet &b) { return a.Mlvl < b.Mlvl; };
    static bool byCLVL(const Planet &a, const Planet &b) { return a.Clvl < b.Clvl; };
    static bool byDLVL(const Planet &a, const Planet &b) { return a.Dlvl < b.Dlvl; };
    static bool byPLVL(const Planet &a, const Planet &b) { return a.Plvl < b.Plvl; };
private:
    int x;
    int y;
    int z;
    int Mlvl;
    int Clvl;
    int Dlvl;
    int Plvl;
    int Mstor;
    int Cstor;
    int Dstor;
};

class Universe
{
public:
    Universe(std::string sName) : sName(sName) {}
    void readCSV(const std::string &filename)
    {
        std::ifstream input(filename, std::ios::in);
        Planet p;
        while (input >> p) {
            vPlanet.push_back(p);
        }
    }

    friend std::ostream &operator<<(std::ostream &out, const Universe &u)
    {
        out << '\n';
        for (const auto &p : u.vPlanet)
            out << p;
        return out;
    }
    void sort(Planet::sortby field) {
        std::sort(vPlanet.begin(), vPlanet.end(), Planet::sorter(field));
    }
private:
    std::string sName;
    std::vector<Planet> vPlanet;
};
#endif

Here is the revised Main to go with it:

Main.cpp

#include "Universe.hpp"

int main()
{
    Universe Uno("Uno");
    Uno.readCSV("Galaxy.txt");

    bool exit = false;
    while (!exit)
    {
        std::cout << "Welcome to the Galaxy Management Unit (GMU).\n\n" 
            "0. Print.\n\n"
            "1. Sort X.\n"
            "2. Sort Y.\n"
            "3. Sort Z.\n\n"
            "4. Sort Mlvl.\n"
            "5. Sort Clvl.\n"
            "6. Sort Dlvl.\n"
            "7. Sort Plvl.\n\n"
            "999. Exit.\n\n"
            "Choice: "
            ;
        int choice;
        std::cin >> choice;
        switch (choice)
        {
        case 0:
            std::cout << Uno << std::endl;
            std::cin.sync();
            std::cin.clear();
            std::cin.get();
            break;
        case 1:
            Uno.sort(Planet::X);
            break;
        case 2:
            Uno.sort(Planet::Y);
            break;
        case 3:
            Uno.sort(Planet::Z);
            break;
        case 4:
            Uno.sort(Planet::MLVL);
            break;
        case 5:
            Uno.sort(Planet::CLVL);
            break;
        case 6:
            Uno.sort(Planet::DLVL);
            break;
        case 7:
            Uno.sort(Planet::PLVL);
            break;
        case 999:
            exit = true;
            break;
        default:
            break;
        }
    }
}

Among the many changes made to the code:

1. Don't hardcode the file name
2. Delegate I/O to the object being read
3. Use C++11 range-for to simplify printing a collection
4. Use const where practical
5. Combine strings into a single one where practical

Answer 5

Using vector to store input record data could become very inefficient within vsSanitizeData(). Since vectors use an array internally, when you remove an element within a vector all of the elements after the one removed must be shifted. For a long list with many remove operations that could be a lot of unnecessary work.

As a matter of design I would sanitize the records as early as possible, within the read loop. There's no point in inserting an unusable record into the list. You may also have other criteria for rejecting a record and that would be a good place to localize those concerns.

As a matter of style I agree with an earlier answer regarding Hungarian notation and the annoyance of it. I believe it is pointless to adorn variable names with type indicators. It is redundant and becomes subject to inaccuracy when code is carelessly refactored over time so it can't be relied upon anyway. Better to have narrative yet simple and concise names that are oriented toward describing the problem domain. For example, instead of Uno I would just use universe, instead of vPlanet I would just use planets.

Good start and keep at it. Coding is recoding.