0
\$\begingroup\$

There are 2 versions of the get_startup_folders() method below, both work as expected and are called only once.

The 1st version has several minor pieces of repeated code which are removed by the use of an inner function in the 2nd version.

I've read several times that the use of inner functions is often frowned on by Python pros, except for in a narrow range of specific cases (e.g. closures and factory functions). I was wondering whether the 2nd version below would be considered superior to the 1st version by experienced Python programmers.

1st version:

def get_startup_folders(self):

    folders = []

    for item in STARTUP_FOLDERS:

        if item == "home":
            if self.home_folder not in folders:
                folders.append(self.home_folder)

        elif item == "file":
            if self.file_folder:
                if self.file_folder not in folders:
                    folders.append(self.file_folder)

        elif item == "project":
            for folder in self.project_folders:
                if folder not in folders:
                    folders.append(folder)

        elif item == "user":
            for folder in self.user_folders:
                if folder not in folders:
                    folders.append(folder)

    # Fail-safe: If empty then use the home folder as a fall back;
    # self.home_folder is guaranteed to contain the home folder.

    if len(folders) == 0:
        folders.append(self.home_folder)

    return folders

2nd version:

def get_startup_folders(self):

    folders = []

    def add_folder(folder):
        if folder and folder not in folders:
            folders.append(folder)

    for item in STARTUP_FOLDERS:

        if item == "home":
            add_folder(self.home_folder)

        elif item == "file":
            add_folder(self.file_folder)

        elif item == "project":
            for folder in self.project_folders:
                add_folder(folder)

        elif item == "user":
            for folder in self.user_folders:
                add_folder(folder)

    # Fail-safe: If empty then use the home folder as a fall back;
    # self.home_folder is guaranteed to contain the home folder.

    if len(folders) == 0:
        add_folder(self.home_folder)

   return folders
\$\endgroup\$
2
\$\begingroup\$

Use a set

Sets guaranty the unicity of the items it holds, adding the same item twice will result in the second add doing nothing:

def get_startup_folders(self):
    folders = set()
    for item in STARTUP_FOLDERS:
        if item == "home":
            folders.add(self.home_folder)
        elif item == "file":
            folders.add(self.file_folder)
        elif item == "project":
            folders.update(self.project_folders)
        elif item == "user":
            folders.update(self.user_folders)

    # Fail-safe: If empty then use the home folder as a fall back;
    # self.home_folder is guaranteed to contain the home folder.
    if not folders:
        folders.add(self.home_folder)

    return folders
    # or if you trully need a list: return list(folders)

If order matters, there is no such thing as OrderedSet, but collections.OrderedDict should be close enough. You just need to handle values that you won't keep:

import itertools
from collections import OrderedDict

def get_startup_folders(self):
    folders = OrderedDict()
    for item in STARTUP_FOLDERS:
        if item == "home":
            folders[self.home_folder] = None
        elif item == "file":
            folders[self.file_folder] = None
        elif item == "project":
            folders.update(zip(self.project_folders, itertools.repeat(None)))
        elif item == "user":
            folders.update(zip(self.user_folders, itertools.repeat(None)))

    # Fail-safe: If empty then use the home folder as a fall back;
    # self.home_folder is guaranteed to contain the home folder.
    if not folders:
        return [self.home_folder]

    return list(folders.keys())
\$\endgroup\$
  • \$\begingroup\$ I'm using a list because the order of the added folders must be maintained. \$\endgroup\$ – mattst Dec 30 '16 at 19:01

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.