2
\$\begingroup\$

I'm learning Python and would appreciate any (and all) feedback to improve myself.

input/output example:

>>> list_of_words = ['deltas', 'retainers', 'desalt', 'pants', 'slated', 'generating', 'ternaries', 'smelters', 'termless', 'salted', 'staled', 'greatening', 'lasted', 'resmelts']

>>> sort_anagrams(list_of_words)
[['deltas', 'desalt', 'slated', 'salted', 'staled', 'lasted'], ['retainers', 'ternaries'], ['pants'], ['generating', 'greatening'], ['smelters', 'termless', 'resmelts']]

Here is the code I wrote, it worked well for the above list. Looking for feedback on complexity, overall design, readability and any other thing that pops to mind when reviewing, thanks!

# process list of strings, construct nested list of anagrams.

list_of_strings = ['deltas', 'retainers', 'desalt', 'pants', 'slated', 'generating', 'ternaries', 'smelters', 'termless', 'salted', 'staled', 'greatening', 'lasted', 'resmelts']

list_of_lists = [] 
temp_list = []

def sort_anagrams(list_of_strings):        
    if len(list_of_strings) > 1:
        temp_list = [list_of_strings[0]] # create temp list to be eventually appended to main list
        popped_list = list_of_strings # list of anagrams to remove from source list after iteration, without deleting source list
        popped_list.pop(0) # remove the first item as it's already in the temp_list
        strings_to_pop = []
        
        for string in popped_list:
            if sorted(string) == sorted(temp_list[0]): 
                temp_list.append(string) # passing anagram test so append to temp_list
                strings_to_pop.append(string) # create list of items to remove popped_list
        
        list_of_lists.append(temp_list) # append temp_list as a list element 
        
        for string in strings_to_pop:
            popped_list.remove(string) # remove all strings that were deemed anagram to remove possible duplications
        
        sort_anagrams(popped_list)    # running the function again on list remainder

    elif len(list_of_strings) == 1:
        temp_list.append(list_of_strings[0])
        list_of_strings.pop(0)
        list_of_lists.append(temp_list) # append temp_list as a list element 
        sort_anagrams(popped_list)
        
    else:
        print('you have reached the end and sorted all anagrams. here is the list of anagram lists: \n \n', list_of_lists)


sort_anagrams(list_of_strings)
```
\$\endgroup\$
1
\$\begingroup\$

I think a much simpler approach would be beneficial to you here, as well as if you were to utilize a dictionary to hold the results as you go along. The below code does not make use of recursion though, so if you were trying to learn about recursion in python then this would not help. But if you're just trying to learn what a simple and pythonic function to solve this problem would look like then I think this will help you. Notably it is much less code, easier to reason about, and does not make use of global variables.

def sort_anagrams2(list_of_strings):
    result = {}

    for string in list_of_strings:
        sorted_string = "".join(sorted(string))

        if sorted_string in result:
            result[sorted_string].append(string)
        else:
            result[sorted_string] = [string]

    return list(result.values())
| improve this answer | |
New contributor
ThePoetCoder is a new contributor to this site. Take care in asking for clarification, commenting, and answering. Check out our Code of Conduct.
\$\endgroup\$
  • 1
    \$\begingroup\$ If they were trying to learn about recursion, they picked the wrong problem :-) \$\endgroup\$ – superb rain Aug 3 at 16:02
  • 1
    \$\begingroup\$ Yes I was hoping to get something along the more pythonic line, I had a feeling this wasn't the best approach :) thanks a lot for the feedback!! \$\endgroup\$ – casuar Aug 4 at 14:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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