275
votes
Accepted
A large part of my code has a major design flaw. Finish it off or fix it now?
If I were in your shoes, I would probably try it this way:
first, finish the current project - at least partially - as soon as possible, but in a working state. Probably you need to reduce your ...
- 203k
232
votes
My boss asks me to stop writing small functions and do everything in the same loop
There are other problems
Neither code is good, because both basically bloat the code with a debug test case. What if you want to test more things for whatever reason?
phoneNumber = ...
- 3,546
227
votes
Accepted
My boss asks me to stop writing small functions and do everything in the same loop
Taking the code examples first. You favour:
if (isApplicationInProduction(headers)) {
phoneNumber = headers.resourceId;
} else {
phoneNumber = DEV_PHONE_NUMBER;
}
function ...
- 39.1k
225
votes
Accepted
Should I refactor the code that is marked as "don't change"?
It seems you are refactoring "just in case", without knowing exactly which parts of the codebase in detail will be changed when the new feature development will take place. Otherwise, you would know ...
- 203k
175
votes
Accepted
Rationale to prefer local variables over instance variables?
What is the objective, scientific rationale to favor local variables over instance variables?
Scope isn't a binary state, it's a gradient. You can rank these from largest to smallest:
Global > ...
- 47.7k
146
votes
Is it the correct practice to keep more than 10 years old spaghetti legacy code untouched without refactoring at all in big product development?
It‘s a question of risk management:
Refactoring a system always creates the risk of breaking something that worked before.
The larger the system, the higher its complexity, and the higher the risk ...
- 75.9k
140
votes
Should I refactor the code that is marked as "don't change"?
Yes, you should refactor the code before you add the other features.
The trouble with comments like these is that they depend on particular circumstances of the environment in which the code base is ...
- 108k
123
votes
Accepted
Writing tests for code whose purpose I don't understand
You are doing fine!
Creating automated regression tests is often the best thing you can do for making a component refactorable. It may be surprising, but such tests can often be written without the ...
- 203k
119
votes
Is there a design pattern to remove the need to check for flags?
The only problem I see with your current code is the risk of combinatorial explosion as you add more settings, which can be easily be mitigated by structuring the code more like this:
if(...
- 27.7k
119
votes
A large part of my code has a major design flaw. Finish it off or fix it now?
Finished IT projects, even faulty ones, are much better than unfinished ones.
Unfinished ones can teach you a lot too, but not as much as finished ones.
You may not see it now, but you get an ...
- 1,703
113
votes
How to encourage a team to refactor
Do not ask management for permission to refactor. It's none of their business. You might as well be asking permission to sharpen a pencil.
Management doesn't understand refactoring. It's not a ...
- 105k
111
votes
Accepted
Does "variables should live in the smallest scope as possible" include the case "variables should not exist if possible"?
No. There are several reasons why:
Variables with meaningful names can make code easier to comprehend.
Breaking up complex formulas into smaller steps can make the code easier to read.
Caching.
...
- 199k
101
votes
Is it the correct practice to keep more than 10 years old spaghetti legacy code untouched without refactoring at all in big product development?
One reason is it's really difficult to measure the loss of productivity the messy code is causing, and difficult to estimate the work it will take to clean it properly and fix any regressions.
The ...
- 147k
82
votes
Rationale to prefer local variables over instance variables?
The original code is using member variables like arguments. When he says to minimize the number of arguments, what he really means is to minimize the amount of data that the methods requires in order ...
- 881
70
votes
Does it ever make sense for a refactor to end up with a higher LOC?
To answer that, let's take a real world example that happened to me. In C# a library that I maintain, I had the following code:
TResult IConsFuncMatcher<T, TResult>.Result() =>
TryCons(...
- 39.1k
69
votes
Is "Parent x=new Child();" instead of "Child x=new Child();" a bad practice if we can use the latter one?
It depends on the context, but I would argue you should declare the most abstract type possible. That way your code will be as general as possible and not depend on irrelevant details.
An example ...
- 57.8k
65
votes
Accepted
When are enums NOT a code smell?
Enums are intended for use cases when you have literally enumerated every possible value a variable could take. Ever. Think use cases like days of the week or months of the year or config values of ...
- 147k
65
votes
Accepted
Where does refactoring belong in GitFlow branch naming model?
Refactoring work should go in a feature branch.
The prefix "feature" is just a word to describe a discrete programming task, you could choose any word you like, any branch from development is either ...
- 72.5k
60
votes
Should I refactor the code that is marked as "don't change"?
My question is: should I refactor the code when I encounter such warnings from the authors
No, or at least not yet. You imply that the level of automated testing is very low. You need tests before ...
- 8,097
59
votes
Accepted
How to write unit tests before refactoring?
You're looking for tests that check for regressions. i.e. breaking some existing behaviour. I would start by identifying at what level that behaviour will remain the same, and that the interface ...
- 4,676
59
votes
My boss asks me to stop writing small functions and do everything in the same loop
There is no "right" or "wrong" answer to this. However, I will offer my opinion based on 36 years of professional experience designing and developing software systems ...
There is no such thing as "...
- 759
58
votes
Getting buy-in for cleaner and more structured code
The 2 years of experience me was you, but more extreme. I'd always create interfaces for every class, I'd apply any design pattern where I was able to, I'd never inject any concrete implementation, ...
- 1,691
56
votes
A large part of my code has a major design flaw. Finish it off or fix it now?
I would happily start the project over.
You're a student, and you're still learning. This puts you in a very different position than the question you linked to.
You have no professional ...
- 677
54
votes
Getting buy-in for cleaner and more structured code
TL;DR - You need Unit Testing
Unit Testing is a phrase which I haven't seen mentioned anywhere else in this thread so far.
(At least, from reading the original post I'd have been willing to bet that ...
- 11.7k
51
votes
Rationale to prefer local variables over instance variables?
Other answers have already explained the benefits of local variables perfectly, so all that remains is this part of your question:
Yet despite these benefits I still cannot seem to convince the ...
- 4,070
51
votes
Is it the correct practice to keep more than 10 years old spaghetti legacy code untouched without refactoring at all in big product development?
It depends on your definition of "correct practice".
I'm currently working on said old spaghetti code, much of it is old enough to drink. It's a critical safety system.
Changes to this code ...
- 663
50
votes
My boss asks me to stop writing small functions and do everything in the same loop
“Entities must not be multiplied beyond necessity.”
— Occam's Razor
Code has to be as simple as possible. Bugs like to hide in between complexity, because they are difficult to spot there. So what ...
- 134k
50
votes
Accepted
In TDD, should I add unit tests to refactored code?
Testing before and after
In TDD, should I add unit tests to refactored code?
"refactored code" implies you are adding the tests after you've refactored. This is missing the point of testing your ...
- 47.7k
48
votes
Which object should have the method?
A user is someone who is registered and able to use the system.
A chat room is a place people can chat. What happens when a user joins a chat room? What is that thing that represents a user who has ...
- 35.1k
47
votes
Unexpected Code Coverage Reduction
The problem I see here is that you have made the code coverage a trigger for build failure. I do believe that code coverage should be something that is routinely reviewed, but as you have experienced,...
- 46k
Only top scored, non community-wiki answers of a minimum length are eligible