Filtering a list of names based on certain conditions

Question

I wrote this code to filter a list of names based on certain conditions, and wanted to see if I can reduce this further and eliminate redundancy in code.

names1 = ["Jane", "Jake", "Bradley", "Bill", "Betty", "Kara", "Kris", "Jil"]
names2 = ["George", "Kate", "Karen", "Kurt", "Greg", "Gary"]
selection_criteria = ["full_list", "four_characters", "three_characters", "start_with_k", "start_with_z"]

def sublist (name_list, condition):
    return_list = []
    if condition == "full_list":
        return name_list
    if condition == "four_characters":
        for name in name_list:
            if len(name) == 4:
                return_list.append(name)
    if condition == "three_characters":
        for name in name_list:
            if len(name) == 3:
                return_list.append(name)
    if condition == "start_with_k":
        for name in name_list:
            if name[0] == 'K':
                return_list.append(name)
    if condition == "start_with_z":
        for name in name_list:
            if name[0] == 'Z':
                return_list.append(name)
    return return_list

for criteria in selection_criteria:
    print(sublist(names1, criteria))

for criteria in selection_criteria:
    print(sublist(names2, criteria))

Answer 1

Overall your code looks fairly clean and well written. Naming conventions are consistent, and your names are clear and spelled correctly. Good job.

• While your naming conventions are consistent, and most of your variable names are good, a few could use some work. for example, the list return_list in the function sublist() could be improved. Don't name a variable for what data type it is, or what its suppose to do. Name the variable for what it is representing. Because a variable is just that. Instead of return_list you should name it something such as filtered_names.

  Also while on the topic of naming, I should mention that criterion is the singular form for criteria. So if you wanted you could be more specfic, you could considered using it.

• Use elif statements instead of multiple if statements. When Python is reading your source code, the virtual machine which runs Python must check every if statement to test its validity, which takes time. If you make use of elif statements instead, as soon as one elif statement is true, Python can jump straight to the return statement in your function. It also adds the benefit of making your code more explicit.

• Now is a good time to suggest a better name for your function. When creating names for functions give them meaningful names. I should just be able to read your function name and have a good idea of what your function is suppose to accomplish. Instead of sublist, which really doesn't convey much meaning to me, use filter_names_by_criteria() or even just filter_by_criteria(). Also, Instead of using name_list, condition for your parameters, I would use names, criterion

• In programming, there is a commonly used principal called Don't Repeat Yourself(DRY). The principal is often useful to apply to most every program. In your case look here:

  ...
  for criteria in selection_criteria:
      print(sublist(names1, criteria))
  
  for criteria in selection_criteria:
      print(sublist(names2, criteria))
  

  Here you running two, practically identical for loops. Instead, extract this logic to a function:

  def print_filtered_names(names, criteria):
      for criterion in criteria:
          print(sublist(names, criterion))
  
  print_filtered_names(names1, selection_criteria)
  ...
  

If you've followed along, and implemented my improvements your code should look something like:

names1 = ["Jane", "Jake", "Bradley", "Bill", "Betty", "Kara", "Kris", "Jil"]
names2 = ["George", "Kate", "Karen", "Kurt", "Greg", "Gary"]
selection_criteria = ["full_list", "four_characters", "three_characters", 
"start_with_k", "start_with_z"]

def filter_names_by_criteria(names, criterion):
    if criterion == "full_list":
        return names
    # don't create a list without knowing if you'll need one 
    filtered_names = []
    if criterion == "four_characters":
        for name in names:
            if len(name) == 4:
                filtered_names.append(name)
    elif criterion == "three_characters":
        for name in names:
            if len(name) == 3:
                filtered_names.append(name)
    elif criterion == "start_with_k":
        for name in names:
            if name[0] == 'K':
                filtered_names.append(name)
    elif criterion == "start_with_z":
        for name in names:
            if name[0] == 'Z':
                filtered_names.append(name)
    return filtered_names 

def print_filtered_names(names, criteria):
  for criterion in criteria:
    print("criterion: {}".format(criterion))
    print(filter_names_by_criteria(names, criterion))

print_filtered_names(names1, selection_criteria)
print_filtered_names(names2, selection_criteria)

Answer 2

I would make this two different functions, one for filtering all names of length n and one for all names starting with some letter. I made them into generator comprehensions, so they are more memory efficient on their own (even though here I cast them to lists, as soon as I want to use them).

Then there is a function which is similar to your function, except that it is configurable. By default it just yields the original list. If additional n's or c's are passed, it applies these criteria.

Finally there is a print function, which has your concrete realization of n = 4, 3 and c = "K", "Z".

def n_chars(names, n):
    return (name for name in names if len(name) == n)

def starts_with(names, c):
    return (name for name in names if name.lower().startswith(c))

def print_sublists(names):
    print(names)
    print(list(n_chars(names, 4))
    print(list(n_chars(names, 3))
    print(list(starts_with(names, "k"))
    print(list(starts_with(names, "z"))

if __name__ == '__main__':
    names1 = ["Jane", "Jake", "Bradley", "Bill", "Betty", "Kara", "Kris", "Jil"]
    names2 = ["George", "Kate", "Karen", "Kurt", "Greg", "Gary"] 
    for names in names1, names2:
        print_sublists(names)

Answer 3

Some notes:

• def sublist (. No space between identifier and parenthesis.
• Better naming: sublist is not declarative enough; names_list: just use the plural form: names.
• Use elif to write a switch construct.
• Use list-comprehensions whenever possible instead of the imperative pattern empty+for+if+append.
• name[0] == 'K': That will fail if name is empty. Use str.startswith.
• Are you sure you want to silently accept an unknown condition name? If not you can raise a ValueError exception on that branch.

I'd write:

def filter_names(names, condition):
    if condition == "full_list":
        return names
    elif condition == "four_characters":
        return [name for name in names if len(name) == 4]
    elif condition == "three_characters":
        return [name for name in names if len(name) == 3]
    elif condition == "start_with_k":
        return [name for name in names if name.startswith("K")]
    elif condition == "start_with_z":
        return [name for name in names if name.startswith("Z")]
    else:
        return []