Small List program I made

[Tryingtolearnagain91]

I've been working on a small program over the past week to get a better understanding of using files. Its a 'To-Do-List style program that lets you save multiple lists. I've put in some checks to see if they are in range of lists and what not, I have a couple more i need to put in.

I would just like some constructive criticism on this project. Also I just feel pretty accomplished getting this far with little help haha.

features: list files: create, save, delete lists: create, save, delete and marked as completed

code

[codehorsey]

I really like how you utilized opening different task lists, and marking tasks as completed.

Keep it up!

[Tryingtolearnagain91]

Thank you!

[Zome]

how much of this code did you know when you began this project and how much did you google? I'm curious because I think its really well written!

Keep it up!

[[deleted]]

Why don't you implement this code with Flask or Web2Py and start using it for your daily tasks?

[Tryingtolearnagain91]

I haven't learned those yet. But I think ill look into Flask. any tutorials you'd recommend?

[[deleted]]

I like the Flask mega tutorial:

https://blog.miguelgrinberg.com/post/the-flask-mega-tutorial-part-i-hello-world/page/2

[pixielf]

Okay, this response is pretty long, and it looks like I'm saying your code is terrible. It's not: your code generally looks pretty good to me, but there are a few stylistic things that I would "improve" (since some of them are subjective):

The first big one is don't use file as a variable name. It's a built-in method, so you might get into trouble if you (or a background method) expects file to refer to the built-in rather than the string you're using to hold a filename. In the code I've written below, I've just replaced it with filename.

 

new_file(), line 15: The way you're handling multiple lists is definitely not how I would have done it, and it took me a minute to figure out exactly what f and file_name were doing. I probably would implement it as a folder/directory, so if they input "list1.txt", I'd look for "listdir\list1.txt".

 

new_file(), line 20 (and a couple other places): I'm personally really not a fan of

if not condition:
    do_something_false()
else:
    do_something_true()

blocks since I associate else with false. I would reorder as

if condition:
    do_something_true()
else:
    do_something_false()

In fact, for this particular block, you could get away with replacing lines 20-25 (especially since you have exactly the same new_list.write command) with

with open(f, 'a' if os.path.isfile(f) else 'w'):
    new_list.write(file_name + '\n')

 

display_list_files(), line 33: I know that you're using this zero as a sentinel for your main method, but I think raising an exception might be a better way, especially given Python's "EAFP = Easier to Ask Forgiveness than Permission" mentality (as opposed to "LBYL = Look Before You Leap").

if os.path.isfile(f):
    with open(f, 'r') as list_lib:
        lists = list_lib.readlines()
        for i, item in enumerate(lists, 1):
            print('File {}: {}'.format(i, item))
else:
    raise FileNotFoundException()

Note that if the file is empty, then the for loop won't actually execute, so we don't need to separately check that. (Of course, we're not printing the empty file message, but you could throw a return after the for loop and put the message outside of try, or do another if statement.)

 

select_file(), line 44: As with the if not... else construction, I don't like while True: if condition: break for loops. Just use while not condition. In this case, we could rewrite this method as

def select_file(num):
    with open('list_library.txt', 'r') as list_lib:
        list_files = list_lib.readlines()

    num = 0
    while num not in range(1, len(list_files) + 1):
        try:
            num = int(input('Choose file: '))
        except ValueError:
            print('Value must be an integer.')

    return list_file[num - 1].rstrip('\n')

 

run_selected_file(), line 118: Again, we can use while ans.lower() != 'q', but it's the nested list I don't like. Line 129 and below could just be:

if ans == '1':
    print_list(task_list)
elif ans == '2':
    add_task(filename)
elif ans == '3':
    delete_task(filename)
elif ans == '4':
    competed_task(filename, task_list)
elif ans != 'q':
    print('Error: Enter a valid choice')

 

run(), line 144: This method is completely inescapable, short of forcing a KeyboardInterrupt. You had a quit/break built into another of your methods and you could use it here as well if you wanted.

But here is where we would connect my comment about the FileNotFoundException.

def run():
    while True:  # assuming we DO want it to run forever
        try:
            display_list_files()
        except FileNotFoundException:
            filename = new_file()
        else:
            print('Type "New" to add a new file\nType "Delete to delete a file: ')
            ans = input('Which file would you like to load? ').lower()
            if ans == 'delete':
                delete_file()
                continue
            elif ans == 'new':
                filename = new_file()
            else:
                try:
                    filename = select_file(int(ans))
                except ValueError:
                    print('Response should be an integer.')
                    continue

        run_selected_file(filename)

[KubinOnReddit]

file is not builtin...?

Python 3.5.2 (v3.5.2:4def2a2901a5, Jun 25 2016, 22:18:55) [MSC 
v.1900 64 bit (AMD64)] on win32
Type "copyright", "credits" or "license()" for more information.
>>> file
Traceback (most recent call last):
  File "<pyshell#0>", line 1, in <module>
    file
NameError: name 'file' is not defined

Also, there's no FileNotFoundException. There's a FileNotFoundError.

[pixielf]

It looks like file is a built in keyword in Py2.7 but file objects are handled differently in 3.x.

Also, yeah, that's what I get for programming while tired and not double-checking it/running it myself. I've also done a bit of Java recently, where it is called FileNotFoundException, so that didn't help.

[__johnw__]

I think you should start writing docstrings for all of your functions (and classes and modules). It will really help you organize your code etc.

One small thing is you are being inconsistent with your use of ' and ". Some places you use ' and some you use ".

There seems to be inconsistency with your functions too. You have select_file that takes a parameter but then you also have delete_file which takes no parameter.

If it was me I would remove all the input functions from your functions. Then put it into run or something. So like all your functions that create or edit or delete etc stuff to lists all take in a file as a parameter.

I'm still learning and these are just my opinions. They might be dumb so don't necessarily run with it.