8

I am writing a physics simulation program in C++. I am a novice in OOP and C++.

In my program, several objects need to be initialized based on data from an input file.

For example, an imaginary input file:

# Wind turbine input file:
number_of_blades = 2
hub_height = 120

# Airfoil data:
airfoil1 = { chord = 2, shape = naca0012}
airfoil2 = { chord = 3, shape = naca0016}

For this example, lets say I have a Turbine class and an Airfoil class. Airfoil objects need to know their chord and their shape, and the turbine object needs to know the height and number of blades.

Should I do this so that each object can build itself from an input file?

e.g.:

class Turbine {
 public:
    Turbine(File input_file);  // reads input file to get the number of blades
 private:
    int num_blades_;
    double height_;
};

or should it be done with a free function:

Turbine create_turbine_from_file(File input_file)
{
    Turbine t;
    t.set_num_blades(input_file.parse_num_blades());
    t.set_height(input_file.parse_height());
    return t;
};

class Turbine {
 public:
    Turbine();

    void set_height();
    void set_num_blades();

 private:
    int num_blades_;
    double height_;
};

What are the advantages disadvantages of each method? Is there a better way?

2 Answers 2

5

First of all, congratulations for taking programming a step further and wondering about how to do it better (and for asking a good question). It's a great attitude and absolutely necessary to take your programs a step further. Kudos!

What you're dealing with here is a problem related to your program's architecture (or design, depending on who you ask). It's not so much about what it does, but how it does it (i.e. the structure of your program instead of its functionality). It's very important to be clear about this: you could totally make those classes take File objects as input, and your program could still work. If you went a step further and added all the exception handling code and took care of edge cases related to files and I/O (which should be done somewhere) in those classes (...but not there), and they became a hodgepodge of I/O and domain logic (domain logic means logic related to the actual problem you're trying to solve), your program could "work". The goal, if you plan on making this more than a simple, one-off thing, should be that it work properly, meaning that you can change parts of it without affecting others, fix bugs as they surface and hopefully extend it without too much difficulty when and if you find new features and use cases you want to add.

OK, now, the answer. First: yes, using Files as method parameters in the Turbine class violates the SRP. Your Turbine and Airfoil classes should not know anything about files. And, yes, there are better ways to do it. I'll talk you through one way I would do it first and then go into more detail about why it's better later. Remember, this is only an example (not really compilable code, but a sort of pseudocode) and one possible way to do it.

// TurbineData struct (to hold the data for turbines)

struct TurbineData
{
    int number_of_blades;
    double hub_height;
}

// TurbineRepository (abstract) class

class TurbineRepository
{
    // Defines an interface for Turbine repositories, which return Vectors of TurbineData structures.
    public: 
        virtual std::Vector<TurbineData> getAll();
}

// TurbineFileRepository class

class TurbineFileRepository: public TurbineRepository
{
    // Implements the TurbineRepository "interface".
    public:
        TurbineRepository(File inFile);
        std::Vector<TurbineData> getAll();
    private:
        File file;
}

TurbineFileRepository::TurbineFileRepository(File inFile)
{
    // Process the File and handle everything you need to read from it
    // At some point, do something like:
    // file = inFile
}

std::Vector<TurbineData> TurbineFileRepository::getAll()
{
    // Get the data from the file here and return it as a Vector
}

// TurbineFactory class

class TurbineFactory
{
    public:
        TurbineFactory(TurbineRepository *repo);
        std::Vector<Turbine> createTurbines();
    private:
        TurbineRepository *repository;
}

TurbineFactory::TurbineFactory(TurbineRepository *repo)
{
    // Create the factory here and eventually do something like:
    // repository = repo;
}

TurbineFactory::createTurbines()
{
    // Create a new Turbine for each of the structs yielded by the repository

    // Do something like...
    std::Vector<Turbine> results;

    for (auto const &data : repo->getAll())
    {
        results.push_back(Turbine(data.number_of_blades, data.hub_height));
    }

    return results;
}

// And finally, you would use it like:

int main()
{
    TurbineFileRepository repo = TurbineFileRepository(/* your file here */);
    TurbineFactory factory = TurbineFactory(&repo);
    std::Vector<Turbines> my_turbines = factory.createTurbines();
    // Do stuff with your newly created Turbines
}

OK, so, the main idea here is to isolate, or hide, the different parts of the program from each other. I especially want to isolate the core part of the program, where the domain logic is (the Turbine class, which actually models and solves the problem), from other details, such as storage. First, I define a TurbineData structure to hold the data for Turbines that comes from the outside world. Then, I declare a TurbineRepository abstract class (meaning a class that cannot be instantiated, only used as parent for inheritance) with a virtual method, that basically describes the behavior of "providing TurbineData structures from the outside world". This abstract class can also be called an interface (a description of behavior). The TurbineFileRepository class implements that method (and thus provides that behavior) for Files. Lastly, the TurbineFactory uses a TurbineRepository to get those TurbineData structures and create Turbines:

TurbineFactory -> TurbineRepo -> Turbine // with TurbineData as a means of passing data.

Why am I doing it this way? Why should you separate file I/O from the inner workings of your program? Because the two main goals of the design or architecture of your programs are to reduce complexity and to isolate change. Reducing complexity means making things as simple as possible (but not simpler) so that you can reason about the individual parts properly and separately: when you're thinking about Turbines, you shouldn't have think about the format in which the files that contain the turbine data are written, or whether the File you're reading is there or not. You should be thinking about Turbines, period.

Isolating change means that changes should affect the least possible amount of places in the code, so that the chances that bugs happen (and the possible areas where they can happen after you change the code) are reduced to the absolute minimum. Also, things that change often, or are likely to change in the future, should be separate from the things that aren't. In your case, for example, if the format in which Turbine data is stored in the files changes, there should be no reason for the Turbine class to change, only classes like TurbineFileRepository. The only reason Turbine should change is if you added more sophisticated modeling to it, or the underlying physics changed (which is considerably less likely than the file format changing), or something similar.

The detail of where and how the data is stored should be handled separately by classes, such as TurbineFileRepository, that will, consequently, have no idea about how Turbines work, or even why the data they provide is needed. These classes totally should implement I/O exception handling, and all the kind of boring and incredibly important stuff that happens when your program talks to the outside world, but they should not go beyond that. The function of TurbineRepository is to hide from TurbineFactory all those details and only provide it with a vector of data. It's also what TurbineFileRepository implements so that no details about it need be known to whoever wants to use TurbineData structures. As a nice possible feature change, imagine you wanted to store turbine and airfoil data in a MySQL database. For that to work, all you'd need to do is implement a TurbineDatabaseRepository and plug it in. Nothing more. Cool, huh?

Best of luck with your programming!

4

It should normally be implemented as a free function. That function should normally be named operator>> and take two arguments: in istream and a reference to a Turbine (and return the istream that was passed to it). In a typical case, it'll be a friend of the class, since it needs to be able to directly manipulate internals that (in many cases) the outside world shouldn't touch (directly).

class Turbine {
    // ...

    friend std::istream &operator>>(std::istream &is, Turbine &t) {
        // Simplifying a bit here, but you get the idea. 
        return is >> t.num_blades_ >> t.height_;
    }
};

This not only satisfies SRP, but makes the class work with the rest of the standard library. For example, if you wanted to read a file full of specifications of Turbines (not just one), you could do something like this:

std::ifstream in("Turbines.txt");

std::vector<Turbine> turbines { 
    std::istream_iterator<Turbine>(in),
    std::istream_iterator<Turbine>()
};
2
  • 2
    This really feels like the Repository Pattern is the more appropriate solution. What if you go from file storage to using a database? Commented Mar 1, 2016 at 2:22
  • @GregBurghardt Repository Pattern is a good idea but it's exclusive with this solution, it could just build on top of it and use this operator internally.
    – kamilk
    Commented Mar 2, 2016 at 22:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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