27

I have this piece of code, for some reason when I try to return the path, I get None instead:

def get_path(dictionary, rqfile, prefix=[]):        
    for filename in dictionary.keys():
        path = prefix + [filename]
        if not isinstance(dictionary[filename], dict):          
            if rqfile in str(os.path.join(*path)):
                return str(os.path.join(*path))
        else:
            get_path(directory[filename], rqfile, path)

Is there a way to solve this?

Tomerikoo
  • 18,379
  • 16
  • 47
  • 61
Andrés
  • 305
  • 1
  • 4
  • 9

2 Answers2

53

You need to return the recursive result:

else:
   return get_path(directory[filename], rqfile, path)

otherwise the function simply ends after executing that statement, resulting in None being returned.

You probably want to drop the else: and always return at the end:

for filename in dictionary.keys():
    path = prefix+[filename]
    if not isinstance(dictionary[filename], dict):

        if rqfile in str(os.path.join(*path)):
            return str(os.path.join(*path))

    return get_path(directory[filename], rqfile, path)

because if rqfile in str(os.path.join(*path)) is False then you end your function without a return as well. If recursing in that case is not the right option, but returning None is not, you need to handle that edgecase too.

Martijn Pieters
  • 1,048,767
  • 296
  • 4,058
  • 3,343
  • Your main point (about returning the recursive call) is correct, but I'm not sure that your suggestion about getting rid of the `else` is correct. There is a branch of the original code (where `dictionary[filename]` is not a dict, but `rqfile` is not a substring of the joined path) where it will keep looping over more filenames. However, there probably does need to be a separate `return` statement at the end of the function, to catch when none of the filenames got matched. It might be appropriate to raise an exception in that case (and then you'd want a `try`/`catch` around the recursion). – Blckknght Oct 06 '13 at 23:46
  • 1
    @Blckknght: I certainly want the OP to think about the possibility there that no `return` is reached. – Martijn Pieters Oct 06 '13 at 23:49
4

While I think Martijn Pieters answer addresses the primary issue in his answer (you need to return from the recursive case), I don't think his suggested code will work right.

You're trying to implement a depth-first search for the rqfile value in the nested dictionary dict. But your current code doesn't handle the recursive case correctly. It needs to respond appropriately if the result is found in one of its recursive calls, or if the recursive call failed to find the target.

Here's what I think you need, with some things renamed or rearranged for clarity:

def get_path(directory, rqfile, prefix=[]):
    for filename, value in directory.items():
        path_list = prefix + [filename]
        if not isinstance(value, dict): # base case
            path = os.path.join(*path_list)
            if rqfile in path:   # Found the file. Do you want to do something
                return path      # with the value here, or is it junk?

        else: # recursive case
            try:
                return get_path(value, rqfile, path_list) # this only returns if 
            except ValueError:                     # the recursion doesn't raise
                pass

    raise ValueError("Requested file not found") # not found here or in children

Example usage:

>>> directory = {"a": "a info",
                 "b": {"c": "b/c info", "d": "b/d info"},
                 "e": {"f": "e/f info", "g": {"h": "e/g/h info"}}}
>>> print(get_path(directory, "h"))
e\g\h
>>> print(get_path(directory, r'g\h'))
e\g\h

If you don't want to raise exceptions when the file is not found, you could also return a sentinel value like None in place of the last line, and check for the sentinel value it in the recursive case instead of the try/except:

 result = get_path(value, rqfile, path)
 if result is not None:
     return result
Blckknght
  • 100,903
  • 11
  • 120
  • 169