Username
Username

Reputation: 3663

Why does this recursive function seem to print the reverse of what I expect?

This is my Python 2.7 script.

import os.path, shutil

def makeFolderName(num, dirBase='./interactive'):
    destDir=    dirBase + '-' + str(num) + '/'
    if os.path.isdir(destDir):
        num += 1
        makeFolderName(num)
    else:
        shutil.copytree(dirBase, destDir)

    print destDir
    return destDir


folderName= makeFolderName(1)

print folderName

This is the printed output.

./interactive-18/
./interactive-17/
./interactive-16/
./interactive-15/
./interactive-14/
./interactive-13/
./interactive-12/
./interactive-11/
./interactive-10/
./interactive-9/
./interactive-8/
./interactive-7/
./interactive-6/
./interactive-5/
./interactive-4/
./interactive-3/
./interactive-2/
./interactive-1/
====./interactive-1/

In the method, I thought this would only print destDir if the value of destDir is not a directory. It instead prints num from its highest value to its lowest.

And folderName's value is the lowest num, not the highest.

I want makeFolderName to return a directory name where num is one higher than what exists in the current directory.

What a I doing wrong?

Upvotes: 1

Views: 86

Answers (3)

Kim
Kim

Reputation: 1686

because you need to return at the point of recursion, which stops the rest of the current call executing:

if os.path.isdir(destDir):
    num += 1
    return makeFolderName(num)

Some other things are worth mentioning:

(1) re-assigning to a parameter is usually worth avoiding, and isn't necessary here. Instead of:

if os.path.isdir(destDir):
    num += 1
    return makeFolderName(num)

you could write more simply:

if os.path.isdir(destDir):
    return makeFolderName(num + 1)

(2) the recursion will fail if you reach the default Python maximum recursion depth, which might happen in a few minutes', hours', days' or weeks' time depending on your application. If you ever have 1000+ ./interactive-n directories you'll get an exception.

Here's an iterative version that doesn't bomb out if there are a lot of directories. It uses a few other Python features to simplify and clarify the code.

Note the more Python-like variable and function names using underscores instead of camelCase.

(3) I've separated the code:

  • to find a new directory name

  • to create the new directory

It makes what you're doing a bit more obvious. If you separate concerns from the outset you'll thank yourself later when your library of functions grows to become a much larger application and you're trying to track down puzzling behaviour.

import os
import shutil


def make_new_dir(num, original_dir='./interactive'):
    new_dir = new_dirname(num, original_dir)
    shutil.copytree(original_dir, new_dir)
    return new_dir


def new_dirname(startnum, stem):
    num = startnum
    while True:
        # Python 2.7-3.5
        name = '{0}-{1}/'.format(stem, num)
        # Python 3.6
        # name = f'{stem}-{num}/'
        if not os.path.isdir(name):
            break
        num += 1
    return name


dirname = make_new_dir(1)
print(dirname)

(4) Performance. Every time you call your original method, it will re-scan the existing ./interactive-n directories from num. Here's a generator-based version that remembers where it's up to and searches from that point:

import os
import shutil


def make_new_dir(num, original_dir='./interactive'):
    find_new_dir = unused_dirname(num, original_dir)
    while True:
        new_dir = next(find_new_dir)
        shutil.copytree(original_dir, new_dir)
        yield new_dir


def unused_dirname(startnum, stem):
    num = startnum
    while True:
        # Python 2.7-3.5
        name = '{0}-{1}/'.format(stem, num)
        # Python 3.6
        # name = f'{stem}-{num}/'
        if not os.path.isdir(name):
            yield name
        num += 1


new_dir = make_new_dir(1)
print(next(new_dir))
print(next(new_dir))
print(next(new_dir))


>>> python makedir.py
./interactive-20/
./interactive-21/
./interactive-22/

(5) Again for performance reasons, you might eventually prefer to check for existing directories (and files) using a glob:

>>> import glob
>>> glob.glob(r'./interactive-*')
['./interactive-1', './interactive-10', './interactive-11', './interactive-12', './interactive-13', './interactive-14', './interactive-15', './interactive-16', './interactive-17', './interactive-18', './interactive-19', './interactive-2', './interactive-20', './interactive-21', './interactive-22', './interactive-23', './interactive-24', './interactive-25', './interactive-3', './interactive-4', './interactive-5', './interactive-6', './interactive-7', './interactive-8', './interactive-9']

Using some of Python's strengths, you can extract out the numbered suffix for each file:

>>> interactives = glob.glob(r'.\interactive-*')
>>> existing_dir_numbers = [int(interactive.split('-')[-1]) for interactive in interactives]

.split('-') splits a string into list elements at each dash. a_list[-1] returns the last element in a list. Since we know we're adding a-nsuffix to our directories, the last element is always a string version ofn, which we convert to a number usingint()`.

>>> existing_dir_numbers
[1, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 2, 20, 21, 22, 23, 24, 25, 3, 4, 5, 6, 7, 8, 9]
>>> max(existing_dir_numbers)
25

So now you could choose ./interactive-26 as the new directory name.

Upvotes: 2

Ken Y-N
Ken Y-N

Reputation: 15009

I think this is the simpler and iterative code that you want:

def makeFolderName(num, dirBase='./interactive'):
    destDir = dirBase + '-' + str(num) + '/'
    while os.path.isdir(destDir):
        num += 1
        destDir = dirBase + '-' + str(num) + '/'

    shutil.copytree(dirBase, destDir)

    print destDir
    return destDir

Here we loop until we find an interactive-num that doesn't exist, then copy the files into that directory and return the highest-numbered directory name.

Incidentally, according to this answer, by default once you get to interactive-1000 Python will blow up if you use the recursive solution as Python has a defined limit to the depth of recursion allowed.


Or, if you dislike repeating the destDir assignment:

def makeFolderName(num, dirBase='./interactive'):
    destDir = ''
    while True:
        if not os.path.isdir(destDir):
            break
        num += 1
        destDir = dirBase + '-' + str(num) + '/'

    shutil.copytree(dirBase, destDir)

    print destDir
    return destDir

BTW, if you have a file, not directory, named interactive-num it will get overwritten unless you add something like:

if not (os.path.isdir(destDir) or os.path.isfile(destDir[:-1]):

Don't forget to remove the '/' when checking for a file!

Upvotes: 1

rodrigo
rodrigo

Reputation: 98348

Your error is that you should return after the recursive call:

    if os.path.isdir(destDir):
        num += 1
        return makeFolderName(num)
    else:
        shutil.copytree(dirBase, destDir)

If you do not do that, then when makeFolderName returns this function will go on and run the following code, that is the print of num. But since it is done after returning from the recursive call, you see the numbers in reverse.

Upvotes: 3

Related Questions