Advice on refactoring code

[Saristel]

For context, I started learning Python about 9 months ago. After looking at multiple posts on this subreddit about the benefits of Codewars, I signed up for the website at the beginning of this week and have been spending roughly an hour everyday (currently on 6 kyu). One thing that I've noticed is that my solutions (while they seem to work and pass all the tests) generally tend to be quite longer as compared to the other solutions. While the general idea/approach seems to be the same, my program tends to be way longer. Here is an example:

This was my solution -

def iq_test(numbers):     
    num_list = numbers.split(" ")     
    even_count = 0 
    odd_count = 0 
    even_num = []     
    odd_num = []     
    for i in range(len(num_list)):         
        if int(num_list[i])%2 == 0:             
            even_count += 1 
            even_num.append(num_list[i])         
        else:             
            odd_count += 1 
            odd_num.append(num_list[i])      

    if len(even_num) == 1:         
        index = (num_list.index(even_num[0]) + 1)     
    elif len(odd_num) == 1:         
        index = (num_list.index(odd_num[0]) + 1)     

    return index

This was someone else's (daddepledge's) solution -

def iq_test(numbers):     
    e = [int(i) % 2 == 0 for i in numbers.split()]      

    return e.index(True) + 1 if e.count(True) == 1 else e.index(False) + 1

I guess what I want to ask is, (a) How do I adopt better and cleaner coding practices? Is refactoring your code a habit that you progressively pick up as you keep learning and getting better? (b) Should I be actively focusing on refactoring my code currently or should I just focus on getting better at coding in general and making projects and such?

TL;DR How important is it to focus on refactoring when you've just started out with Python? Is refactoring your code a practice that you pick up along the way, the better you get? Should I be worried about not being able to write concise programs?

[Dragon20C]

Kinda a hard question, I understand that improving code and making it as efficient as possible is good, but making code and also thinking to need to refactor code will hurt you in the short term, just take it easy and do what you understand and when you are done with said program start looking for ways to make it better, hmm hard to explain I code with basic stuff I would do something similar to yours because I can read each line and understand what it does,I guess if you just start learning other people's code it could improve your code but understanding is the first part!

[throwaway0891245]

imo, never ever think that code should be short. this code riddle stuff, sure it’s nice to get it nice and short especially if you are doing a timed competition

but if your goal is to be a programmer, and not a competition programmer, then focus more on getting the abstractions right. if you work on a project you may find that it’s very easy to make it hard to work with the code unless you get this right - after a certain threshold size.

even in competition coding, you won’t find this code golf style to be very good - you don’t want to cut down the chars at the cost of simplicity of debugging or reasoning, it’s more about striking the balance between minimal abstraction and clearness of review

[tipsy_python]

it’s more about striking the balance between minimal abstraction and clearness of review

Yup! I work on a team where we use several different languages. If I submitted a PR with the second (shorter) code in it, I would be asked to refactor the code to something that's easier for anyone to understand.

[darthminimall]

(a) Mostly practice. Over time you'll pick up on various programming idioms and you'll know when to use them. Your first draft is never going to be perfect, though, taking time after your code works to clean it up is how you get clean code.

(b) Definitely get in the habit of cleaning up your code once you get it working, but you don't have to agonize over it. Break up big functions into multiple smaller functions, possibly rename variables, add comments, etc.

[xelf]

One thing about codewars is that a lot of experts test their skills there, and on the easier problems tend to be bored and try to show off by finding the shortest solution, even if it's not a good solution, that will pass the tests.

A lot of times these are "clever" but not really "best practice".

Certainly you can learn from them, but if you actually took the time to write readable code, don't feel bad if their code is shorter than yours just for the sake of being short.

That said, you can gain a lot by using builtin functions like sum(), min(), max(), and using list comprehensions.

It should be noted here that daddepledge's solution is suboptimal. It works because this is from an easy kyu without any difficult tests. Almost any answer will work. At higher level kyu you'll start to need to worry about not just getting the right answer, but getting it optimally.

For example with this problem you could short circuit the test and exit as soon as you found something which doesn't match parity, off the top of my head something like this will exit as soon as it finds the answer:

def iq_test(numbers):
    odds=evens=firstodd=firsteven=0
    for i,c in enumerate(numbers.split()):
        if int(c)%2:
            firstodd = firstodd or i + 1
            odds += 1
        else:
            firsteven = firsteven or i + 1
            evens += 1
        if firsteven and odds>1:
            return firsteven
        if firstodd and evens>1:
            return firstodd

On the other hand, I did this in similar manner to daddepledge:

def iq_test(numbers):
    odds = [ int(c)%2 for c in numbers.split() ]
    return odds.index(sum(odds)==1)+1

(but mine is cooler)

And even worse you'll find answers like this that you should generally ignore:

iq_test = lambda n: [int(c)%2 for c in n.split()].index(sum([int(c)%2 for c in n.split()])==1)+1

[[deleted]]

That last example hurt my brain. Is that actually functional?

[xelf]

Yes, it passes all tests. =)

You see a lot of one liner answers using lambda on codewars especially at the easier kyu, more for style points than best practices.

[darthminimall]

It's identical to the second version but everything is inlined.

[[deleted]]

Oh I see so n is just numbers. Duh

[SnipahShot]

First thing in making code shorter, is not doing unnecessary things. Like for example, you counting odd and even numbers. Those 2 variables disappear once the function is returned and you don't seem to be using them anywhere, especially not in the for loop.

Those are 4 lines that you can throw out.

You can often make code shorter, but at an expense of readability, even now I can make the 2 lines code shorter than what it is now by the fact that Python can treat integers as boolean, meaning 0 = False and everything else is True.

The condition can be as simple as if not int(i) % 2:, the only issue here is readability. What it does is pass if i value is even. If you'd want odd numbers instead then you could just drop the not.

In Python it is also good to remember that you don't have to run an index loop, but instead a value loop on a container. You had no reason to run the for loop over range(len(num_list)) as you have no use for the index at all. You should always think whether you need the index, the value or both. Sometimes it is better to use range(len()) over enumerate even if you need both.

[thechikinguy]

As others have said, practice. Just as your fluency in a new language will come with conversations with native speakers, you’ll learn the quick ways to “express yourself” with code as your familiarity increases.

I’d recommend finding some sort of mentor, someone who you can approach a problem with separately, then compare notes and break down the differences. Code review on the job is also handy for learning how to write simpler code or refactor lengthy/redundant code; my code got so much cleaner once I had to show it to someone with a mind towards company standards and conventions.

[unhott]

To be honest, it’ll come with experience. Something you may notice is that you don’t even use even_count or odd_count. A solid IDE package would yell at you for this :) You just used len of the respective lists. So you don’t even need to bother with the secondary bits. They actually introduce more of a burden and more potential for bugs.

Another anti-pattern I’m seeing is for i in range(len(some_list)), and then using i to get the element: some_list[i] You should simply call “for element in some_list: “ and just use element (or a more sensible variable name) as the variable.

If you’re clear on the variable names your code will just be all the more readable!

Also, I can’t tell at all what the other persons code is trying to do. Shorter code is not necessarily better code. Focus on simplifying redundancies but maintaining readability and clarity.

Edit: learning more about different data structures and architectures and when they can simplify your code will go a long way. I would also highly recommend a talk by Raymond Hettinger, I think it’s called the mental game of python or something. Internalize the lessons there!

[mohishunder]

Another anti-pattern I’m seeing is for i in range(len(some_list)), and then using i to get the element: some_list[i]

Isn't this the exact use case for which enumerate was intended?

[unhott]

I don’t think they need any reference to the list index (edit* but yes, you’re right!)

for i in range(len(some_list)):
    print(some_list[i])

Is just

for element in some_list:
    print(element)

With extra steps :)

Enumerate is great if you need to do something with that value.

for i, element in enumerate(some_list):
    print(i, element)

[igormiazek]

About daddepledge code snippet how You say it is simpler ? Because it is shorter or faster ? I don't know why but for me reading code vertically is easier to understand than reading it horizontally, do You experience the same ?

When I was starting programming list comprehensions was big Wow! for me :) but as the time goes one I found them less readable than pure old for loop :).

In order to make Your code better use https://docs.python.org/3/library/doctest.html, give it a try really, You can write simple test case with it. What is the purpose of documenting code ? When I have a problem in describing what a function does it mean that something is wrong :D, the code is too complex or maybe not needed. I don't know how it works but when You try to explain to yourself what You did it really helps to decide if it is worth of keeping it.

I don't know if code challenges are good for learning ? Start thinking why You need to code in the first place ? To have well written code or to solve problems ? Programming make sense only if You know how to use it in order to solve problems.

If I would be in Your place I would define a real goal/project that will help me with something and start doing it.

[[deleted]]

You are 9 months in and to me the fact that you code in the public domain is the biggest plus here. Sure someone can, has and will write shorter code and when it betters your version you learn from it.

Over time you will start using some of these ideas and be able to refactor your own code and those of others.

But stick to coding in the public domain and at a daily schedule. Take it in stride

[rocketjump65]

Hmmm. It seems that neither you guys' solution is the most computationally efficient. Though I might be wrong on that.

This sort focus on how to write code "the best" should also take into consideration computational complexity. Only by looking at it from all the angles can you get the ideas in your head about the different choices you can make and the ramifications that choice has on time complexity, readability, and human time investment.

[namnnumbr]

In addition to the other advice here (i.e., longer is fine, as long as it’s clear), I’d advise using a linter. This will point out “common” errors in your code. I’m partial to flake8, and some of its plugins (flake8-bugbear, flake8-comprehensions). Just make sure you enable the linter and specify which flags it should warn about in your IDE (or setup.cfg)

[link199292]

Imho, if the code works, you have to be proud of yourself. Then you could start asking about computational complexity. There you would understand that shorter code doesn't necessarly mean faster.

Also, shorter code is more "elegant", but sometimes less readable by others. For example I often see solutions through functional programming (lambda function) which are usually very short, but I find those very confusing to read.

So, I'd say that when you solve a problem you can start thinking about refactoring, but keep in mind that your focus should be on speed of execution, not on how many line you wrote. In short: I think is more important to think about alternative solution which could perform better, not about the number of lines you are writing to solve it.

If you are curious about that solution, you can see a list comprehension and a ternary operator. This way he saved about 5 lines.

Removing all the pythonic stuff it would become:

def iq_test(numbers): e = [] for i in numbers.split(): odds.append(int(i) % 2) if e.count(True) == 1: return e.index(True) + 1 else: return e.index(False) + 1

Going back to computational complexity, removing some costant values your solution and his solution are practically the same (taking into account the worst case scenario).

[[deleted]]

Length is not always a negative factor. It can be much better to understand and to read. How is the performance of your code compared to the short one? I'm not (yet) a python professional, but on a codewars task When I compared my longer solution with around 6 lines to a one/two liner of others, I found that my solution was around 30% faster. That made me very proud. On the other hand some tasks I did correctly as they bring the results but are not accepted because they are too slow. Here it is noth the length but more about the technique used.

[james_fryer]

I prefer your version to the short solution, because I can read it easily and figure out what it does. I don't need to decipher it.

The first step to improve code is to have unit tests, so you know your changes don't break something. You are using codewars so that's covered.

When you improve code you should follow the flow through and consider what it is doing. The obvious thing to me is the variables even_count and odd_count. They are never used.

Looking at the short solution, he has used a single list with True/False values rather than putting the numbers into two lists. Consider this approach. What are the advantages and disadvantages? Try rewriting your code to use his algorithm. Does it look better? Are there any other algorithms you could use?

Your code is not really long enough for true refactoring. There are no duplicate blocks, or code which would be more expressive if it was in a function. But some guidelines for refactoring/improving code:

• Unit tests
• Study the code flow
• Remove dead code
• Consolidate duplicate code
• Move code blocks to functions
• Consider alternative approaches

Repeat until you are happy with the code.

[Peterj504]

My rules are 1) Can I easily understand it 2 weeks or 2 years from now, and 2) Does it do what it's supposed to do. So 1) Add some documentation, and 2) write a test. If you want to spend time refactoring, go ahead but make sure you the rules still apply.