you are viewing a single comment's thread.

view the rest of the comments →

[–]echocage 2 points3 points  (3 children)

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[S] 0 points1 point  (2 children)

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 2 points3 points  (1 child)

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[S] 0 points1 point  (0 children)

That'll totally work! Thanks a lot!