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?

Was it helpful?

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.

Licensed under: CC-BY-SA with attribution
scroll top