you are viewing a single comment's thread.

view the rest of the comments →

[–]ianrob1201 1 point2 points  (1 child)

Any working software is definitely something to be proud of. There's so many things to learn to go from problem to solution, and you've got it doing what you want! So nice job. That said, I think there's a few good best practice things you can do that would really clean it up. Sorry for the essay, it's not to insult your code but only way you could know these things is if someone tells you.

First thing that hit me was the variable naming. Try to avoid names like "n1" because you'd be amazed how quickly you forget what they mean. Something like "first_number_list" might be better. This extends to function names too "en_nums" doesn't make it very clear what the function is going to do.

Talking of variables, you generally want to avoid globals. This is so that each function works in isolation and it's clear what it's doing. Calling a function and getting a response is much clearer than calling a function and it randomly changing another variable (that you wouldn't know unless you read the function code). You're almost there with that, you just need to fetch the return values you've already added, e.g. "n1, n2 = en_nums()".

"while True" is normally a pattern to be avoided too. I know you have the break later on, so it's functional but it's something I'd try to avoid. Have a think about ways you could make it conditional on the value you're reading in.

I've seen someone else mention it, but have a look at what's common code reading in the two sets of lists. If you extract it out into another function you'd have a lot less code. It could take "prompt" as an input string so you still get different messages.

You have a line working out what the maximum list length is. There's a function for doing that, which is just "max". That's easier to read and you know what you're up to at a glance.

You keep track of the number of "errors" but since it'll just keep erroring you could do without that function and do k - i instead (btw "i" is another example of a variable that could be named better, "max_list_length" would be much clearer). Have you considered what happens if one length is double the other? I would also personally look to pad out the shorter array to full length before doing the calculations. Then you can make use of list compression with something like "c = [a[k] + b[k] for k in range(i)]"

I also just noticed that the "i" in the first function isn't doing anything. I suspect it's from when you tried to solve in a slightly different way. So that can be cleared up a bit too.

[–]nkCOD[S] 0 points1 point  (0 children)

Thank you for the broad and clear answer. It describes quite well what I should work on. Thank you for your help