0

I've got to refactor this code as seeing from SOLID viewpoint it is not a good practice(not extensible), what are the alternatives?

I have a text file from which i read the commands, its format goes like:

ADD_CHILD name1 name2 gender

GET_RELATIONSHIP name1 relation ..

The problem here i think is when i pass words to the function because if the format of text file changes my code will break. One thing that comes to mind is use of if-elif statements but they are not advised by open-closed principle. What are other possible approaches?

    path_str=input("Enter the path of input file ")
    fileinput=open(path_str,"r")
    for line in fileinput:
        words=line.split()
        function=set1solution.function_select_map.get(words[0])
        result=function(family,words)

    function_select_map={
        "ADD_CHILD":add_child,
        "GET_RELATIONSHIP":get_relationship
    }

    relation_map={
        "Paternal-Uncle":Family.get_Paternal_Uncle,
         ......}

    def add_child(family,words):
        #find person just returns an object of person class
        parent=family.find_person(words[1])
        new_addition=Person(words[2],words[3],words[1])
 result=family.add_external_child(words[1],new_addition.name,new_addition.gender)
        return result

    def get_relationship(family,words):
        person=family.find_person(words[1])
        function=set1solution.relation_map.get(words[2])
        result=function(family,person)
        return result

1 Answers1

0

As you correctly mentioned, your code depends on file format that is not recommended. add_child and get_relationship shouldn't know about file format, so you should have following data flow in your code:

File => File parser => app logic

File parser is some kind of mediator between data in file and your code. There are few possible ways to achieve this, here is part of one short solution (with my comments):

def add_child_parser(words):
    return add_child, (family, words[0], words[1], words[2])  # return params for add_child function in correct order, not that it's not obvious from your code where family come from

def command_parser(line):
    d = {
        "ADD_CHILD": add_child_parser,
        "GET_RELATIONSHIP": get_relationship_parser
    }  # dict with all availbale command parsers
    words = line.split()
    curr_parser = d[words[0]]  # get parser for current command
    func, args_list = curr_parser(words[1:])  # call parser for current command with other words as params
    func(*args_list)  # call returned function

def file_parser(file_path):
    file_input = open(file_path, "r")
    for line in file_input:
        command_parser(line)  # process each line

def add_child(family, parent_name, child_name, child_gender):  # note, we receive all required data in params, not as single string
    parent = family.find_person(parent_name)
    new_child = Person(child_name, child_gender, parent_name)

    return family.add_external_child(new_child)
ingvar
  • 4,169
  • 4
  • 16
  • 29