What types of anti-pattern coding do you always refactor when you cross them?

Question

I just reworked some code that was in another section of the class I was working on, because it was a series of nested conditional statements (? :), which was made more subtle with the help of a fairly simple switch statement (C #).

When will you touch code that is not what you are working on to make it more understandable?

Answer 1

I once was refactoring and came across something like this code:

string strMyString; try { strMyString = Session["MySessionVar"].ToString(); } catch { strMyString = ""; } 

Resharper pointed out that .ToString () is redundant, so I pulled it out. Unfortunately, this led to a code violation. Whenever MySessionVar was null, it did not raise a NullReferenceException, which he was referring to raise it to the catch block. I know it was some kind of sad code. But I learned a good lesson from it. Do not quickly go through the old code, relying on a tool that will help you do refactoring - think about it through yourself.

I ended up refactoring as follows:

 string strMyString = Session["MySessionVar"] ?? ""; 

Update: Since this post is supported and does not technically contain an answer to a question, I decided that I should answer this question. (Well, it bothered me to such an extent that I really dreamed about it.)

Personally, I ask myself a few questions before refactoring.

1) Is the system under source control? If so, continue with refactoring because you can always back down if something breaks.

2) Are there unit tests for the functionality that I am modifying? If so, great! Refactoring The danger here is that the presence of unit tests does not indicate the accuracy and scope of these unit tests. Good unit tests should pick up any violations.

3) Do I fully understand the code I'm refactoring? If there is no source control and no tests, and I really don't understand the code I'm changing, this is a red flag. I would need to be more comfortable with the code before refactoring.

In case # 3, I will probably take the time to actually keep track of all the code that currently uses the method I'm refactoring. Depending on the size of the code, this may be easy or impossible (that is, if it is a public API). If it comes to the public API, you really need to understand the initial purpose of the code from a business perspective.

Answer 2

I will only reorganize it if the tests are already in place. If not, it's usually not worth my time to write tests and refactoring, apparently working code.

Answer 3

This is a small, insignificant antipattern, but it annoys me so much that when I find it, I immediately delete it . In C (or C ++ or Java)

 if (p) return true; else return false; 

becomes

 return p; 

In the scheme

 (if p #t #f) 

becomes

 p 

and in ML

 if p then true else false 

becomes

 p 

I see this antipattern almost exclusively in code written by students . I definitely do not do this !!

Answer 4

Whenever I come across this, and I don’t think that changing it will cause problems (for example, I can understand it enough so that I know what it is doing, for example, the level of voodoo is low).

Answer 5

I just bothered to change it if there is some other reason why I am changing the code.

How willing I am to accept this depends on how confident I am that I will not break anything and how extensive my own changes in the code are.

Answer 6

This is a great situation to demonstrate the benefits of unit tests.

With unit tests in place, developers can boldly and aggressively reorganize the oddly written code that they may encounter. If it passes unit tests, and you increase readability, then you have done your good deed during the day and can move on.

Without unit tests, simplifying the complex code filled in by voodoo poses a great risk of code breaking and not even knowing that you introduced a new error! Therefore, most developers will take a cautious route and move on.

Answer 7

For simple refactoring, I try to clear deeply nested control structures and really long functions (more than one screen for text). However, this is not a great idea for refactoring code without good reason (especially in a large development team). In general, if refactoring does not make a big improvement in the code or fixes a blatant sin, I try to leave enough alone.

Not refactoring for one, but just as general housekeeping, I usually do this when I start working on a module:

• Delete stupid comments
  • Comments that say nothing more than a function signature already say
  • Comments that are pure idiocy similar to "just the way it looks"
  • List of changes at the top of the file (we have version control for some reason)
  • Any API documents that are clearly not in sync with the code
• Delete commented code snippets
• Add version control tags, such as $ Id $, if missing
• Fix problems with spaces (this can annoy others because your name is displayed for a large number of lines in the difference, even if all you did was change the space)
  • Remove space at the end of lines
  • Change the tabs-> spaces (for this there is our agreement where I work)

Answer 8

Usually I don’t reorganize the code, if I just look through it, I don’t actively work on it.

But sometimes ReSharper points out some things that I just can't resist Alt + Enter. :)

Answer 9

If the refactor makes the code much easier to read, duplicate code, for example, will be the most common for me. if / else, which differs only in the first / last commands.

 if($something) { load_data($something); } else { load_data($something); echo "Loaded"; do_something_else(); } 

Answer 10

More than (possibly) three or four lines of duplicated code always makes you think about refactoring. In addition, I tend to move the code a lot, extracting code that I suppose will be used more often in a separate place - a class with its clearly defined purpose and responsibility or a static method of a static class (usually placed in my Utils. * Namespace).

But in order to answer your question, yes, there are many cases where the code is shorter in short, does not necessarily mean that it is well structured and readable. Another example is using an operator ?? in c #. What you need to think about is new features in your chosen language - for example, LINQ can be used to do some things very elegantly, but it can also make a very simple thing very unreadable and overly complex. You need to weigh these two things very carefully, in the end it all comes down to your personal taste and, mainly, experience.

Well, that’s another answer “depends”, but I’m afraid it should be.

Answer 11

I almost always break 2 similar conditional expressions into a switch ... most often with respect to enums. I will shorten the return instead of a long statement.

Example:

  if (condition) {
   // lots of code
   // returns value
 } else {
   return null;
 } 

becomes:

  if (! condition)
   return null;

 // lots of code ..
 // return value 

debugging early reduces extra padding and reduces long bits of code ... also, as a general rule, I don't like methods with more than 10-15 lines of code. I like methods that have a special purpose, even if you create more private methods internally.

Answer 12

I try to reorganize very long functions and methods if I understand the set of inputs and outputs to a block.

This facilitates readability.

Answer 13

I would only reorganize the code that I encounter and do not actively work on the following steps:

• Talk to the author of the code (not always possible) to find out what this part of the code does. Even if it’s obvious to me what part of the code does, it always helps to understand the rationale for why the author may have decided to do something in a certain way. Spending a couple of minutes talking about it will not only help the original author understand your point of view, but also create trust within the team.
• Know what this part of the code does to test it after repeated factoring (the build process with unit tests is very useful here, which makes all this quick and easy). Perform unit tests before and after the change and make sure that nothing breaks due to your changes.
• Send commands to the team (if you are working with others) and inform them of upcoming changes, so no one is surprised when the change actually occurs.

Answer 14

Refactoring for this is one of the roots of all evil. Please never do this. (Unit tests soften this somewhat).

Answer 15

Are the extensive lines of commented code antipattern? Although it is not a specific code (he comments), I see a lot of this behavior with people who do not understand how to use source control, and who want to keep the old code for later, so that they can return more easily if an error was introduced. Whenever I see extensive snippets of code that are commented out, I almost always delete them entirely.

Answer 16

I am trying to reorganize based on the following factors:

• As far as I understand, to know what is happening?
• Is it possible to easily return if this change violates the code.
• Will I have enough time to go back if it breaks the assembly.

And sometimes, if I have enough time, I reorganize to study. Like in, I know that my change may break the code, but I don’t know where and how. Therefore, I still change to find out where to break it and why. This way I will learn more about the code.

My domain was mobile software (cell phones), where most of the code is on my PC and does not affect others. Since I also support the CI build system for my company, I can run a complete build of the product (for all phones) with the updated code to ensure that it does not break anything. But this is my personal luxury, which you may not have.

Answer 17

I tend to reorganize global constants and enumerations quite a bit, if they can be considered a low risk of refactoring. Perhaps this is so, but something like the ClientAccountStatus enumeration should be in the ClientAccount class or next to it, and not in the GlobalConstAndEnum class.

Answer 18

Exclude / update comments that are clearly incorrect or clearly meaningless.
Removing them:

• safe
• version control means you can find them again
• improves code quality for others and for you.

This is the only 100% risky refactoring I know.

Please note that doing this in structured comments like javadoc comments is another matter. Changing them is unsafe, therefore they can be fixed / deleted, but some guaranteed corrections will not be fixed, since there will be standard incorrect code comments.