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 Turbine
s 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 File
s. Lastly, the TurbineFactory
uses a TurbineRepository
to get those TurbineData
structures and create Turbine
s:
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 Turbine
s, 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 Turbine
s, 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 Turbine
s 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!