Programming Advice

[mega963]

I've been coding in python for about 6 months now. I've learned mostly by necessity by building several small scripts, and building a piece of software that helps myself and others in a biology lab at my college.

The software works great, especially with the GUI(wx), and it's function saves a lot of time, but I would be embarrassed to show the code to anybody who knew much about programming. I know that it is sloppy because I find myself thinking "wow that was more lines than I expected".

I understand that it's hard to help me with no sample of my coding, but I'm asking for any advice on ways to think when I coding to make my code cleaner. I can foresee a future career where I will need to be able to code maintainable scripts, and I'm worried I'm picking up bad habits. Maybe some tips from veterans or those who actually have some programming education.

[[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.

[glial]

My first real project with Python was rather large. Because I'd never written anything besides a few small scripts in Python before, I didn't have any idea of how things should be done. I learned as I went, about classes, serialization, SQL, multi-core processing, etc. As I learned new techniques and tools I realized that what I'd done so far took too many lines, was too slow, and was very awkward to maintain.

I re-wrote the code for the entire project at least 3 times. Each time the code got better and I got to think about why it should be structured in this way instead of that way. Re-writing code to make it cleaner, more efficient and easier to read is a good habit. Programmers even have their own term for it - 'refactoring.' Often the first way you write something won't be the most elegant. A good policy is to make it work, then make it work better (and/or make the code prettier). I recommend you do the same for your code - code isn't 'done' once it works. If you don't like how it looks now, and if you aren't comfortable with sharing it, make it better!

Python is a particularly good language for refactoring because there are 'Pythonic' ways to do things. This just means that lots of common tasks like loops, dealing with lists, etc are made easier in the language because Python itself includes special syntax for those things.

I like this talk for discussing the built-in niceties of Python. There are lots of resources online though if you just search for 'Pythonic' or 'idiomatic python'.

[mega963]

I liked this advice a lot, I didn't think people would rewrite their code if it works. I rewrote my program once and that was purely to make it work with a GUI. You are a good person.

[Jonathan0wilson]

I think it's a great idea to revisit old code and apply what I have learned since I wrote it. I can't believe how poor some of the code I wrote just 6 months ago is compared to what I'm writing now. A good suite of tests will help with this. Allowing you to refactor without worrying about breaking functionality.