Should a constructor parse input?

Question

Often, I find that I must instantiate a bunch of objects, but I find it easier to supply the parameters for this instantiation as a human-readable text file, which I manually compose and feed into the program as input.

For instance, if the object is a Car then the file might be a bunch of rows, each containing the name, speed and color (the three mandatory constructor parameters) delimited with tabs:

My car          65       Red
Arthur's car    132      Pink
Old junk car    23       Rust brown

This is easy for me to inspect visually, modify or generate by another program. The program can then load the file, take each row, parse out the relevant parameters, feed them into a Car(string name, int speed, uint color) constructor and create the object.

Notice how there is some work that must be done on the input before it is compatible with the constructor: The speed must be converted from string to int with a call to int.Parse. The color must be matched to a RGB value by looking up the English color name (perhaps the program would access Wikipedia to figure out each color's value, or consults a predefined map of name -> RGB somewhere).

My question is, from an OOP standpoint, who should do this parsing? The constructor, or the method calling the constructor?

With the first option, the advantage is simplicity. The calling function must only do:

foreach(var row in input_file)
     list_of_objects_that_i_am_populating.Add(new Car(row));

And all the ugly parsing can be nicely contained in the constructor, which doesn't have much other code anyhow, so the parsing code can be easily read and modified without being distracted by non-parsing code.

The disadvantage is that code reuse goes out the window because now my object is joined at the hip to an input format (worse, because the input format is ad-hoc and manually composed, it is ephemeral and potentially not guaranteed to stay the same). If I reuse this object in another program, where I decide that it is convenient to slightly change the formatting of the input file, the two versions of the object definition are now divergent. I often find myself defining input formats in the comment section of the constructor, which seems a bit code-smelly.

Another disadvantage is that I have lost the ability to do batch operations. Recall the earlier example problem of mapping color names to values: What if I was using a web service that takes 1 minute to process every individual request, regardless of whether that request is asking to convert one color name or a million. With a very large input file, I would drastically slow down my application by accessing the service once for each row, instead of submitting one big request for all rows, and then instantiating the objects according to the reply.

What is the "correct" way to handle a situation like this? Should I parse input the constructor and treat the problems above as exceptional issues that must be dealt with on a case-by-case basis? Should I let my calling method do the parsing (even though it may already be bloated with much convoluted program logic)?

Answer 1

My question is, from an OOP standpoint, who should do this parsing? The constructor, or the method calling the constructor?

In general, you should avoid doing this within the constructor. That would be a violation of the Single Responsibility Principle. Each type should only be responsible for the operations required within that type, and nothing else.

Ideally, a separate class would be responsible for parsing the data into its proper form (and nothing else). The method creating your instance would take that (parsed) data and create your types.

Answer 2

I would create and use factory methods to load via a settings/file,csv and NOT put such code in the constructor itself.

Factory version 1:

public class Car
{
     ... your existing methods and data ...


     public static Car CreateFromCsv(string csv ) { .... }
     public static Car CreateFromFile(string fileName) { ...}
 }

Or use a dedicated Factory:

public static class CarFactory
{
   public static Car CreateFromCsv(string csv ) { .... }
   public static Car CreateFromFile(string fileName) { ...}
}

Or a dedicated business logic class:

namespace BusinessLogic;

public class LoadCars
{

    public Car ExecuteForCsv(string csv) { ...}
    public Car ExecuteForFile(string fileName) { ... }
}