2

I am writing a finite element software in C++ and Qt. I want to load the input data to the model. I have a Model class that holds the data and does the work. I have created a simple lexer and parser to read the input file (things that resemble database records). I don't have the experience to make an informed decision on how relate the file reader class and model class. I have two options:

1) In class Model, create a method to load the file, i.e. in pseudocode:

 class Model
 {
   // private fields

   public:
     loadFile( filename )
  }

2) This:

class Lexer(input filename)
{
    ...
    loadToModel(Model&)  // red the input filename into the Model.
}

Which one is better or makes more sense? What are the pros and cons of each one?

Alex Reinking
  • 1,607
  • 11
  • 16
Ring Zero.
  • 181
  • 7

1 Answers1

5

Although the code in your question isn't very clear, my understanding is that you are proposing to create an object from some input data. You outline two options:

  1. Make loading the data a public method. There are few upsides to this beyond convenience. It's a pretty clear violation of the Single Responsibility Principle and it couples the model's in-memory representation to its on-disk representation. This isn't usually a very good thing. It also potentially leaves your model in an invalid state after creation, which is also very error-prone. It is much better to have classes that are never in invalid states whenever possible. A slight improvement would be to make this a static member function, rather than an instance method.

  2. Pass an existing instance to a separate loader. This is a somewhat better approach if, again, your model object has no invalid states. And if your model has no invalid states, then it should be trivial to initialize a default-constructed one inside the function, rather than taking a reference as a parameter.

The best solution, then, is the adjustment I mentioned in (2). Design your model so that a default-constructed instance is valid, and then write a method in your Lexer (perhaps renamed to "ModelLoader" or "Parser" since it is doing more than lexing) to load a file from disk and populate a fresh Model instance, and then return it. In modern C++, guaranteed return value optimization (RVO) should alleviate any concerns you might have with returning a large object.

Ideally, your loader/deserializer would also take an input stream of some sort, rather than a filename directly. Qt's QDataStream, for example, can abstract over several different media (drives, network, in-memory buffers, etc.)

If you need (or want) your Model to be immutable, then you can employ the Builder pattern to stage updates to a Model before actually constructing it.

Alex Reinking
  • 1,607
  • 11
  • 16
  • But isn't (1) the usual way to serialize,/deserialize ? And why is there a violation of SRP ? – Christophe Nov 15 '18 at 08:49
  • 2
    No, it isn't. Even extending the basic std io streams involves writing non-member specializations. That's also the way Qt's `QDataStream` does it, which OP says they're using. But the violation of SRP here is the fact that `loadFile(filename)` implies that Model is not only responsible for deserialization but _thirdly for disk I/O_. – Alex Reinking Nov 15 '18 at 09:21
  • SRP is about reason to change and not about functionality. Here an explanation of Uncle Bob who invented the SRP: https://blog.cleancoder.com/uncle-bob/2014/05/08/SingleReponsibilityPrinciple.html . – Christophe Nov 15 '18 at 10:09
  • 2
    @Christophe and where you are sourcing your sequences of bytes is a changable choice, with almost no relation to what you are doing with those bytes – Caleth Nov 15 '18 at 10:16
  • You can very well have an archiver that decides how to read/write the data (see [boost](https://www.boost.org/doc/libs/1_68_0/libs/serialization/doc/index.html) approach) but have the serialisation done in the class (injecting the archiver), in order to avoid dependencies and hidden coupling (i.e. knowing what to read/write). Don't get me wrong: I'm not against decoupling loading from data format (on contrary). I'm just saying that the object itself is in the best position to know what should be persisted to restore its state. – Christophe Nov 15 '18 at 10:28
  • 1
    @Christophe: it pretty much depends on what the responsibility of the `Model` class should be here in this case: if it is just a dumb container, persisting logic could fit into its realms of responsibility. If it contains also finite element calculation logic, then this would give at least two very "different reasons to change" to that class. – Doc Brown Nov 15 '18 at 11:07
  • @Christophe: and about the SRP: the SRP is first and foremost about responsibility. Uncle Bob did IMHO not invent it, he just invented the "one reason to change" metapher - which is just a metapher, no less, no more. – Doc Brown Nov 15 '18 at 11:27
  • @DocBrown it's a recurring discussion between us ;-) is there any older reference to this principle ? Or any other author that claimed that a class should do only one thing (why should a class then be allowed to have several methods) before uncle Bob ? (I instisted on class because for functions there are). R.C.Martin even wrote somewhere that he didn't mean the responsibility OF the class but the responsibility FOR the class (i.e. the designer in charge of deciding what the class shall look like). So IMHO, SRP created a big misunderstanding due to a successful book and an unfortunate wording – Christophe Nov 15 '18 at 11:43
  • The https://blog.cleancoder.com/uncle-bob/2014/05/08/SingleReponsibilityPrinciple.html is very fascinating. These decisions on the design of software can lead to success or failure. That is why I posted this question to learn from you. :) – Ring Zero. Nov 15 '18 at 19:12
  • Also, I intend to do all my finite element stuff in the model. Model is going to have methods that do the matrix calculations and matrix assembly and different solvers. Should I worry about this as well ? It feels natural to me that each model should have its solver as a method in it. – Ring Zero. Nov 15 '18 at 19:39
  • 1
    Many thanks to all. I think I want to go with a separate IO class. It makes a cleaner design. I am really thankful for all the comments and answers. – Ring Zero. Nov 15 '18 at 20:46