Does creating a list in the constructor violate the guideline that a constructor shouldn't do work?

Question

I was reading these pages (1,2,3), but I'm still unsure if this violates the guideline.

I have the following data being read from a website:

Date: July 13, 2018 
Type: Partial Solar Eclipse
Location: South in Australia, Pacific, Indian Ocean
Date: July 27, 2018 
Type: Total Lunar Eclipse
Location: Much of Europe, Much of Asia, Australia, Africa, South in North America

An object is constructed that represents the data, and the client can call methods to do work with the data.

In my object the locations are left as a string, because it's easier to search, for example:

class Eclipse():
 def __init__(self, eclipse_location):
 # left out other variables and validation to keep it clear and concise
 self._eclipse_location = eclipse_location
 def takes_place_in(self, country):
 country_formatted = country.strip().title()
 return country_formatted in self._eclipse_location

This way no matter what other words surround the countries, for example "Much of" or "Parts of" it will return True. If the locations were split in a list I would have to create a string and join the country to create "Much of insert country here" and run a search for that string. Also, depending on which part of the site is scraped, the phrase "Much of" isn't always used, sometimes it gets more specific, for example, South in Australia, which means my method has to be changed to accommodate south (and most likely north,east,west). I know because I wrote the method both ways, and keeping it as a string is easier and I still get the behavior I want.

Suppose if I want to create a list of locations, would calling this method violate the guideline that a constructor shouldn't do work?

class Eclipse():
 def __init__(self, eclipse_location):
 # left out other variables and validation to keep it clear and concise
 self._eclipse_location = eclipse_location
 self._locations_list = self._locations_as_list(self._eclipse_location)
 def _locations_as_list(self, str_to_convert):
 return str_to_convert.split(",")

After the object is created, the list doesn't change, and no methods exist to modify (add or remove) the list. To me anyways, it makes sense to create the list once in the constructor, and call the list when ever I need it. If I don't create the list in the constructor anytime I need a list of locations I would have to call _locations_as_list(self, str_to_convert):. This seems inefficient, for a small list it maybe fine, but what if that list contained 100+ elements?

• Robert Harvey: It's not about how long it takes to construct the object, or how long the list is, if the object requires the list in order to function. It's more about whether parsing data should be a responsibility of this particular object, or whether that responsibility should be delegated to some other object. I would argue that parsing is a separate responsibility, demanding a separate class. I don't know how this works out in Python, but in other languages like C# or Java your class's constructor would be handed a fully hydrated list, not some data to be parsed.
• Telastyn: Building a list isn’t too bad. Parsing input totally is though.
• Flater: Note that a more literal reading of the title (creating a list = instantiating an empty list) renders a different result. Instantiating an empty list is fine. Executing logic that populates the list is not.
• user306112: @RobertHarvey - In your opinion, does putting logic to create and populate the list the way I'm suggesting, violate SRP?
• Robert Harvey: I would argue that your class should probably be handed the completed list in the constructor, as I explained in my first comment.

Answer 1

Yes. It would be bad practice to put your string to list logic in the constructor.

There's plenty of things that can go wrong parsing that string and you have little opportunity to deal with errors when the logic is in the constructor.

Consider what it would be like if you made class which parsed location strings to list.

I would include the following functions:

class LocationParser():
 def IsValid(str_to_convert) #return true if the string can be parsed.
 def Validate(str_to_convert) #return a list of validation errors such as :
 #"illegal character found at pos x", 
 #"unknown country!", 
 #"duplicate country"
 def Parse(str_to_convert) #return the list of locations

Both functions might throw exceptions such as for null or very long strings

now you can create a second version of the object to deal with different languages, or expand the functions so, for example, you can pass in a list of delimiters to use rather than comma.

 def Parse(str_to_convert, delimiters) #return the list of locations

If you tried to put all that logic into your constructor you would face a number of problems, how to return the validation errors or pass in the set the delimiters, how to change the logic out for a differently formatted string etc

Using a separate object or a even a method on the Eclipse object allows you to override the behaviour either through inheritance or composition, or by simply keeping it separate.

• Deduplicator: Splitting a string doesn't seem to have all that many opportunities to fail; the only one I see is OOM...
• Ewan: sure, if its just splitting a string. But you already have 'parts of' and 'most of', next there will be a location with a comma in it etc, nulls etc. The best design is clear, whether its worth the time depends on a whole tonne of stuff
• Ewan: "YAGNI" the cry of the optimist. It's next to no effort to put it its own class, and the times you do need it, it saves your ass
• Robert Harvey: This would be a better answer if it didn't read like a Monty Python's Flying Circus episode (cue "Ministry of Silly Walks" skit).
• Ewan: no, A. that would be a global variable and B. static is always bad. You could inject the parser and call a method yes, but it would be simpler just to parse the string externally and pass the resulting list into the Eclipse constructor. There are several different ways of doing it, choose the one the best fits the style of your code