There was a good discussion on Twitter yesterday regarding a code contribution to the Laravel framework. It ended with some good questions about ...
For further actions, you may consider blocking this person and/or reporting abuse
Refactoring should also aim to make something either 1) more readable, 2) more performant (because of an actual need to make it more performant), or 3) more flexible to changes that have happened in the past. The code change above fails all 3. It's harder to understand what's happening thus making it less flexible to changes that may be needed in the future.
There is nothing special about a 1 liner (it's what I call "clever coding" always with the quotes, and it's a really bad practice). A computer can read 10 lines really really quickly, and it generally takes as long or longer for a human to decipher what the 1 liner actually does vs the 10 line equivalent.
I couldn't agree more about one-liners. Also using "tricks", little-known features and other "clever" stuff usually just makes the code harder to comprehend. IMHO the code example above doesn't really need refactoring anyway.
Great comment, some people still/will continue to think that "one line" is a huge performance optimization
If its against a test suite, its refactoring full-stop. Insufficient tests are a separate concern.
That's the "gray-area" I want to discuss in my follow up post. 😉
I will read OFC,
I think for me the iterative nature of anything we want to do means having crap tests (consistency first) is a more build with foundations approach for those starting out or unsure what the tests points should be.
When I started to code in the 90's nobody showed me tests; nobody paying me asked for them. It was mid 00's before I realized I had to retro-fit my skills with an essential component.
Of course there's always room for improvement red->green->refactor, people over processes, but the kernel of development has to be testing something, not necessarily testing the right thing. Finding that right thing takes
time and experience. Even then we all often get it wrong 😉
Interesting point.
IMO it's the only responsibility of the contributor to check if the changed code is sufficiently covered by tests to make his change a refactoring. This is true regardless if he is the only contributor in the project or one of many.
It's like being careful when crossing a street even if the lights are green.
I've definitely done this in the past. I'd too excited to after "refactor"-ing a method to a one-liner and overlooked the fact that I've made that method destructive. Great post!