CODEWITHSUNDEEP
refactoring
user513788
user513788

Reputation: 103

Just because something works, does it mean you don't have to refactor?

I just saw this line of code in the WP codebase.

if ( $user_id = wp_validate_auth_cookie( '', apply_filters( 'auth_redirect_scheme', '' ) ) ) {
}

Um. Yeah. two method calls and an assignment statement in an if statement.

So, my guess is, nobody is refactoring this.

Are there any reasons why?

Upvotes: 3

Views: 198

Answers (3)

Carl Manaster
Carl Manaster

Reputation: 40336

To answer the question in your title:

Just because something works, does it mean you don't have to refactor?

If it did, we'd never refactor. Because refactoring happens on green. That is, it happens when all tests are passing, when - by definition - the code works. If you "refactor" on red, you're not refactoring. You're playing with your code. You're taking risks that you probably shouldn't be taking. Refactoring is improving the design of existing code, without changing its behavior. If your code isn't working, if you don't have tests that demonstrate that it's working, then the changes you make could be changing its behavior.

Upvotes: 5

Marcus Adams
Marcus Adams

Reputation: 53830

This is certainly breaking some coding standards, but is it breaking the WP coding standard?

I wouldn't have an assignment in the condition of an IF block. People will always wonder, is that supposed to be == instead of =?.

Refactoring is good to earn back some of your technical debt. Unit tests and good code coverage makes refactoring easier.

A good rule of thumb is any time you touch the code (or have to stare at it to decipher it), that's the time to refactor it, so the next person doesn't have to deal with it. In this case, you looked at the code, you should refactor it. Of course, these are guidelines that the group has to agree on.

Ideally, you should keep your technical debt under control.

Upvotes: 3

charliegriefer
charliegriefer

Reputation: 3382

Time is a luxury that not everybody has. I'd -love- to go back and refactor just about every line of code I've ever written. We're always learning (hopefully), and could probably always go back and refactor things we've written to be more concise, more elegant, more performant, etc.

But the reality of it is... if it works, there's not usually a high priority to go back and "fix" it. Chances are you're working on something new, and the client doesn't really care if you've got 2 method calls in a block of code where one will do. They want their product.

If the code in question is so non-performant that the system is unusable, they'll let you know (trust me), at which point you'll definitely be doing the refactoring.

Upvotes: 2

Related Questions