5
\$\begingroup\$

I made this python script to create dated folders within a directory. This is my first practical automation project and I don't want to cement any bad habits so I was wondering if there is anything I can improve in the code I have written here. Any feedback will be appreciated!

The main challenge I had was to account for months with different day ranges, and then to reset the counts properly for all the variables. I feel like this is more complicated than it probably needs to be but I don't know if there are some python libraries I am unaware of that could help simplify it more.

I thought about having a predefined end date as well but that would require the user to open up a calendar and find 2 dates. I figured by just asking for a start date and how many folders they want, I could make things simpler for the end user but I'm not even sure that that is actually the case.

I also don't know if I've used classes correctly here. At first I had everything thrown into the while statement. I also considered using a for loop instead of while but went with while instead. Again, this is my first real project so any help with other decisions I could have made will be appreciated!

import os import pathlib directory = "/Users/myName/Box Sync/SAFAC/Budget Requests/2020-2021/Weekly/" # directory = input("Enter the path where you would like to create files: ") date = input("Enter the starting date in mm-dd-yyyy format: ") period = int(input("Enter the period between each date folder: ")) instances = int(input("How many folders do you want? ")) Folders = [] arguments = date.split("-") dayArgument = arguments[1] monthArgument = arguments[0] yearArgument = arguments[2] init = instances - instances elapse = period - period seperator = "-" month = int(monthArgument) day = int(dayArgument) + int(elapse) def make_directories(input_list): for folderName in input_list: dirpath = os.path.join(directory, folderName[0:],'Documentation') try: os.makedirs(dirpath) except FileExistsError: print('Directory {} already exists'.format(dirpath)) else: print('Directory {} created'.format(dirpath)) def add_folder_to_list(month,day,year,extra): full_name_constructor = (month,day,year,extra) # full_name_constructor = (str(month),str('0'+str(day)),yearArgument,"Budgets") fullName = seperator.join(full_name_constructor) Folders.append(fullName) def format_and_append_folder_name(): if day < 10 and month >= 10: add_folder_to_list(str(month),str('0'+str(day)),yearArgument,"Budgets") elif day >= 10 and month < 10: add_folder_to_list(str('0'+str(month)),str(day),yearArgument,"Budgets") elif day < 10 and month < 10: add_folder_to_list(str('0'+str(month)),str('0'+str(day)),yearArgument,"Budgets") else: add_folder_to_list(str(month),str(day),yearArgument,"Budgets") def check_month(): global maxDays if (month % 2 ) == 0 and month != 2 : # if even but not February maxDays = 30 elif month == 2 : maxDays = 28 #if February else : maxDays = 31 # if odd while init < instances : check_month() format_and_append_folder_name() make_directories(Folders) # print(Folders[init]) # for debugging init += 1 elapse += period day = int(dayArgument) + int(elapse) if day > maxDays : month += 1 day -= maxDays # reset days dayArgument = day # recalibrate the new zero point for the day to be the date in the new month elapse = 0 # recalibrate elapse point to 0 so it can reiterate the period again 
\$\endgroup\$
1
  • \$\begingroup\$ Excellent question, particularly for a newcomer to the site. \$\endgroup\$ Commented Jan 8, 2021 at 16:01

4 Answers 4

3
\$\begingroup\$

OK let's look at your code.

Unused Imports

pathlib was imported but I don't see any use of it in the codebase, I understand that this might be as a result of testing out approaches to a problem. pylint can help mitigate this by catching unused modules. It also catches some PEP guidelines which your code breaks.

Avoid Globals

You have lots of global variables. A common approach whenever you want to use a global is asking yourself some questions; "How do I express myself with code whilst avoiding this global variable". When in doubt, prefer objects that holds state to global variables.

Code Review

On first glance, I can see you are manually parsing dates. This doesn't look good, you introduce bugs when reinventing the wheel. Right now, your code contains a bug, it accepts invalid dates, I can easily create a date like this date = "12-03-202022" The above code generates no error, this can be disastrous. A common alternative is using python datetime module. This simplify everything, we can move the dates by periods easily, creating directories becomes quite easy.

NOTE: datetime module checks for invalid dates.

A simple way of creating a date from a string is given as

from datetime import datetime date = datetime.strptime(start_date, "%d/%m/%y") 

This simple line of code simplifies everything, the full code can then be written as

"""Directory creating program""" import os from datetime import datetime, timedelta def next_date(date, period): """Increase the date by timedelta date: datetime object period: timedelta to increase our date by """ new_date = date + timedelta(days=period) return '/'.join((str(new_date.day), str(new_date.month), str(new_date.year))), new_date def make_directories(home_dir, start_date, instances, period): """Create sub directories in the home_dir home_dir: base directory start_date: date to append to directories created instances: number of directories to create period: difference between two dates """ date = datetime.strptime(start_date, "%d/%m/%y") for i in range(instances): start_date = start_date.replace('/', '-') dirpath = os.path.join(directory, start_date,'Documentation') try: os.makedirs(dirpath) except FileExistsError: print('Directory {} already exists'.format(dirpath)) else: print('Directory {} created'.format(dirpath)) start_date, date = next_date(date, period) if __name__ == "__main__": directory = "/Users/myName/Box Sync/SAFAC/Budget Requests/2020-2021/Weekly/" make_directories(directory, '3/1/20', 10, 5) 
\$\endgroup\$
3
\$\begingroup\$

Hard-coded paths

It looks like you started off asking for this:

directory = input("Enter the path where you would like to create files: ") 

but then changed your mind:

directory = "/Users/myName/Box Sync/SAFAC/Budget Requests/2020-2021/Weekly/" 

The latter style of hard-coded directories should be avoided. Accept the directory via input, or (more likely) via argparse.

Unit specificity

Enter the period between each date folder 

is not specific enough. Period in days, months, years? A combined timedelta format? Based on your code it looks like days, but your prompt should specify.

Datetime parsing

Don't do split-based date parsing like this:

dayArgument = arguments[1] monthArgument = arguments[0] yearArgument = arguments[2] 

Instead, call into one of the parsing routines in datetime. Beyond that, mm-dd-yyyy is both non-standard and non-sortable. Strongly consider using yyyy-mm-dd instead.

Likewise, rather than

def add_folder_to_list(month,day,year,extra): full_name_constructor = (month,day,year,extra) # full_name_constructor = (str(month),str('0'+str(day)),yearArgument,"Budgets") fullName = seperator.join(full_name_constructor) 

just use the built-in datetime format routines.

Pathlib

You import it, but you don't use it here:

 dirpath = os.path.join(directory, folderName[0:],'Documentation') 

Best to replace your join with a / operator on a Path instance.

Spelling

seperator -> separator

\$\endgroup\$
2
\$\begingroup\$

Bugs

There are a few (potential) bugs in you program: check_month() doesn't handle leap years and the code doesn't handle roll over at the end of a year.

datetime

Using datetime from the standard library can fix these bugs and simplify the code. datetime.date() provides methods for parsing and formating date strings. And datetime.timedelta() for adding an interval or period to a date.

Using the library, your code can be simplified to something like:

from datetime import date, timedelta from pathlib import Path template = input(f"Enter a directory template (use DD for day, MM for month, YYYY for year): ") template = template.replace('YYYY', '{0:%Y}').replace('MM', '{0:%m}').replace('DD', '{0:%d}') start = input(f"Enter a start date (yyyy-mm-dd): ") current_date = date.fromisoformat(start) period = input("Enter the number of days between each date folder: ") period = timedelta(days=int(period)) count = int(input(f"Enter the number of folders to create: ")) for _ in range(count): path = Path(template.format(current_date)) try: path.mkdir(parents=True) except FileExistsError: print(f'Directory {path} already exists.') else: print(f'Directory {path} created.') current_date += period 

Using a template provides some flexibility in telling the code what directories to create. The template gets turned into a format string by replacing codes in the template with formatting codes for the components of a date. For example, 'YYYY' in the template gets replaced with '{0:%Y}'. When the format string is used to format a date, the {0:%Y} gets replaced with the 4-digit year. 'DD' and 'MM' are use for the day and month respectively. Letting the user enter a code like 'YYYY' and then replacing it with the format code is just to sanitize the user input.

The date.fromisoformat() parses a date in the YYYY-MM-DD format and returns a date object. If you really need to use DD-MM-YYYY format, also import datetime from the datetime library and use start = datetime.strptime(start, "%d-%m-%Y").date().

I prefer using pathlib, so I used that in the code. os.path would work just as well.

User experience (UX)

With some additional effort, the user experience can be improved. For example, default values can be provided for some inputs. As another, a stop date can be used instead of a number of periods (quick: how many weeks are there between 2021-01-08 and 2021-08-01?).

from datetime import date, timedelta from pathlib import Path DEFAULT_TEMPLATE = "/Users/myName/Box Sync/SAFAC/Budget Requests/2020-2021/Weekly/DD-MM-YYYY-Budgets/Documentation" DEFAULT_START = dt.date.today() DEFAULT_PERIOD = 7 # days DEFAULT_COUNT = 1 # a str print("Enter the following (defaults are in [])") template = input(f" Directory template (use DD for day, MM for month, YYYY for year)\n[{DEFAULT_TEMPLATE}]: ") if template == '': template = DEFAULT_TEMPLATE template = template.replace('YYYY', '{0:%Y}').replace('MM', '{0:%m}').replace('DD', '{0:%d}') start = input(f" Start date (yyyy-mm-dd) [{DEFAULT_START}]: ") if start: date = dt.date.fromisoformat(start) else: date = DEFAULT_START period = input(f" Number of days between each date folder [{DEFAULT_PERIOD}]: ") if period == '': period = DEFAULT_PERIOD period = dt.timedelta(days=int(period)) end = input(f" Number of folders or a stop date (yyyy-mm-dd) [{DEFAULT_COUNT}]: ") if end == '': end = date + period * DEFAULT_COUNT elif '-' in end: end_date = dt.date.fromisoformat(end) else: end_date = date + period * int(end) while date < end_date: path = Path(template.format(date)) try: path.mkdir(parents=True) except FileExistsError: print(f'Directory {path} already exists.') else: print(f'Directory {path} created.') date += period 

In the code, the prompts include the default value in '[]'. If the user just hits ENTER, the default values get used. The code for determining the end_date uses the default number of periods if the user doesn't enter anything. Otherwise, it checks to see if the user entered a date or a count and does the appropriate conversion.

\$\endgroup\$
1
\$\begingroup\$

I agree with most if not all the points made by other answers thus far so I won't rehash their answers. Here are some additional points.

  • Use type hints. They will save you (or the next person who has to look at this script) in the long run.
  • Using Clean Code principles, break down responsibility into smaller functions.
  • Check to see if the file that already exists is indeed a folder.
# Trying to make a folder and catching the exception when it already exists # is a good idea except when the thing that already exists is a FILE not a FOLDER. def make_directory(value: str): try: os.makedirs(value) except OSError as e: if e.errno == errno.EEXIST and os.path.isdir(value): pass else: raise 
  • Avoid unnecessary actions. Use init = 0 instead of init = instances - instances
  • if day < 10 and month >= 10: can be written as if day < 10 <= month:
  • If you're using globals, it's a good sign you should create a class.
\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.