-3
\$\begingroup\$

I have written the following function for a CLI tool, it works as expected but it also contains duplicated code.

def populate_teams_tab(self):
    teams = self.get_data(self.TEAMS_URL)
    returned_list = []
    for team in teams:
        if self.IS_ROGUE:
            if team['id'].upper() == "ROGUE":
                # Here I have a few lines of manipulation that are adding all
                # the ROGUE teams to a list that will be returned
                # I will simplify those steps for clarity using the following line
                returned_list.append(team)
        else:
            # Here I have the _same_ lines of manipulation that are adding all
            # the teams to a list that will be returned
            # I will simplify those steps for clarity using the following line
            returned_list.append(team)
        return returned_list

Is adding another helper function the best idea or maybe is there a more elegant solution to get rid of the duplicated part?

\$\endgroup\$

closed as off-topic by 200_success, Heslacher, yuri, IEatBagels, pacmaninbw May 24 at 15:50

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – 200_success, Heslacher, yuri, IEatBagels, pacmaninbw
If this question can be reworded to fit the rules in the help center, please edit the question.

  • 1
    \$\begingroup\$ What does this code accomplish? The function doesn't return anything! See How to Ask. \$\endgroup\$ – 200_success May 24 at 12:39
  • \$\begingroup\$ I've added the precisation. \$\endgroup\$ – Pitto May 24 at 12:50
  • 1
    \$\begingroup\$ But what does this function do? What class does it appear in? \$\endgroup\$ – 200_success May 24 at 13:06
  • \$\begingroup\$ Sorry but I do not understand how this can be useful for the question or for future readers. Putting +100 more lines of code will not add any value to the question and just increase the complexity without a real advantage. The problem is not repeating code that is basicaly data wrangling. Adding it wouldn't change the question in any way. I also already received the answer I was looking for. \$\endgroup\$ – Pitto May 24 at 13:24
  • \$\begingroup\$ At the least, fix the title to conform to the How to Ask guidelines. \$\endgroup\$ – 200_success May 24 at 13:25
1
\$\begingroup\$

We don't have the lines of codes you say they are doing the "same" thing. So I should guess.

If those lines of codes have the same logic, just with different data, It's a great idea to separate those lines of codes in another method.

But it seems that those lines just need teams data, not the data that belong to the object. If that is the case, you should separate those lines in a static method, in python words, you need to add @staticmethod at the top of that method before def keyword and you should remove self from parameters.

But it is always a great idea to export the same codes in a separate reusable place.

\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.