Does creating a list in the constructor violate the guideline that a constructor shouldn't do work?
https://softwareengineering.stackexchange.com/questions/372080
-
06-02-2021 - |
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?
Solution
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.