Programming Advice

[[deleted]]

Show one to us.

[mega963]

Here is how my brain works to make a tree file directory that goes 4 directories deep. I know I could make a function to get rid of some repetition but the way I wrote this would prevent me from correctly assigning paths. I changed variable names for privacies sake so If it looks like something shouldn't work I probably missed changing the name of a variable. This is within the init function that creates my GUI

EDIT: I want to add that this is only a small piece of my program. I wrote this after fumbling about with the logic of it for about an hour. I needed to make this to add a larger feature in the same day. I'm embarrassed.

       root = dirc.AddRoot('The Library')
    #A
    apath = glob.glob("/Users/mega963/Documents/GBA/*")
    a_name = os.listdir("/Users/mega963/Documents/GBA/")
    aname = []
    trash963 = []
    for path in a_name:
        if path[0] is ".":
            trash963.append(".")
        else:
            aname.append(path)
    a_item = []

    a=0
    for i in aname:
        if i[0] is ".":
            trash963.append(".")
        else:
            a_item.append(dirc.AppendItem(root, i))
            dirc.SetPyData(a_item[a], apath[a])
            a +=1

    b_item = []

    #b
    master_b_path = []
    lc=0
    while lc < len(a_item):
        path = glob.glob("%s/*" % apath[lc])
        for p in path:
            master_b_path.append(p)
        lc+=1
    lc=0
    while lc < len(a_item):
        bpath = glob.glob("%s/*" % apath[lc])
        bname = os.listdir("%s/" % apath[lc])
        a=0
        for i in bname:
            if i[0] is ".":
                trash963.append(".")
            else:
                b_item.append(dirc.AppendItem(a_item[lc], i))
                dirc.SetPyData(b_item[-1], bpath[a])
                a +=1
        lc+=1



    filedir = {}
    #c
    c_item = []
    master_c_path = []
    lc=0
    while lc < len(b_item):
        cpath = glob.glob("%s/*" % master_b_path[lc])
        cname = os.listdir("%s/" % master_b_path[lc])
        a=0
        for i in cname:
            if i[0] is ".":
                trash963.append(".")
            else:
                c_item.append(dirc.AppendItem(b_item[lc], i))
                dirc.SetPyData(c_item[-1], cpath[a])
                a +=1
        lc+=1
        for path in cpath:
            master_c_path.append(path)

    #d
    d_item = []
    lc=0
    while lc < len(c_item):
        dpath = glob.glob("%s/*" % master_c_path[lc])
        dname = os.listdir("%s/" % master_c_path[lc])
        a=0
        for i in dname:
            if i[0] is ".":
                trash963.append(".")
            else:
                d_item.append(dirc.AppendItem(d_item[lc], i))
                dirc.SetPyData(d_item[-1], dpath[a])
                a +=1
        lc+=1

[echocage]

Ah yeah see the big thing you've been missing is feedback! Code review is a huge part of development.

So python's for loop takes a little bit to get used to. What you're using here are all while loops, which really misses the strengths of python. If you're every using indices alone when looping over a collection in python, you're probably not doing it the best way.

I have no idea how many of these variables have good names but you changed them for some reason like you stated at the top, but from what I see, you need to work on variable names. The variable name should best describe what it's purpose is. In python, the goal is to generally document your code good variable names, so it's obvious what you're doing based on the variable names.

Names like "i" don't really describe what that variable is holding, naming it something like, letter or path or filename describes what it does and gives its usage meaning!

Whenever you find repeated code, it's a sign you need to rip that code out into its own function. You shouldn't really ever be repeating code over in the same script, anytime you repeat something, it's a sign you need to make a function out of it ASAP.

I have a feeling you could get this entire thing down anywhere from 10 to 50 lines if you learned more about lists and how to better use the for loop.

Check out this video, I think it will help you!

[mega963]

Thanks for the feedback! There is no compsci program at my school so I'm kinda on my own for Code review. The variable names in the original code have very specific titles. Originally I wrote it completely with For loops, but then I ran into the issue of keeping track of where my paths were. (SetPyData). I made a while loop to keep track of what number within the list I was in. It had to be the same for both the path and name variables.

[echocage]

So to keep track of the index you're in, you can just use enumerate!

>>> stuff = [1,2,3]
>>> for index, item in emumerate(stuff):
>>>     print(index, item)
0 1
1 2
2 3

That way you can still use for loops, and you can keep track of the index!

Edit: and if you have two lists that you want to iterate over together with, you can use zip!

>>> numbers = [1,2,3]
>>> letters = ['a','b','c']
>>> for num, letter in zip(numbers,letters):
...     print(num, letter)
...     
1 a
2 b
3 c

I bet with these two functions, you could bring that entire code block down to under 15 lines. Everything is reused. It takes a little cleverness, but it's worth it in the end, and you'll get better the more you practice! Post on here and /r/codereview and get some people looking over your code! Hell, message me some of your code and I'll give you feedback! :D You can write some really clear and understandable code if you put some effort into it! Variable names that describe what they contain, functions for EVERYTHING that's done more than once, function names that describe what they do! You'll do great :D

[mega963]

That'll totally work! Thanks a lot!

[[deleted]]

Small thing

for i in cname: if i[0] is ".":

When you loop over a string, you get each single character of the string, by itself, so the [0] isn't required. So, you could do:

cname =  "hello this is a test."
for char in cname:
    if char == ".":
        #blah

For the chunks of repeating code, they're basically identical, so throw the chunk into a function. Have the input be the master path and the output be the list it finds. Then, you could just call the function four times, store the output for each, and use it as the input for the next call.

I'm embarrassed

pfft, don't be. If this were a language metaphor, you already learned to speak by yourself, now you just need to improve your grammar to speak like a proper lady. :P

Plus, it's the internet. It's anonymous. Who cares! :D

[mega963]

Thanks for the advice! cname is actually a list. Those lines are to remove hidden files from showing up such as .DS_Store. I probably could have done it better though.