A commonly used term is "Magic numbers". As discussed in a related question, this is considered a code smell. I assume the same would go for string constants, although the term "Magic strings" is not in common use. So to generalize a bit, it would seem that ALL constants should be named, thus making them "unmagical". Or am I missing som...
Is there anything wrong with the below code? It is java codes, but somehow it looked like a C program to me. If there is something incorrect with the OO implementation, can you tell me the name of the violation? looking at S.O.L.I.D or CodeSmell but I still have no idea why the below codes looked weird to me. public void root() { B...
We are developing what is becoming a sizable ASP.NET MVC project and a code smell is starting to raise its head. Every controller has 5 or more dependencies, some of these dependencies are only used for 1 of the action methods on the controller but obviously are created for every instance of the controller. I'm struggling to think of a...
The codebase where I work has an object called Pair where A and B are the types of the first and second values in the Pair. I find this object to be offensive, because it gets used instead of an object with clearly named members. So I find this: List<Pair<Integer, Integer>> productIds = blah(); // snip many lines and method calls voi...
A friend of mine has just posited that protected methods (yes, methods) constitute a code smell. That is, they're indicative of potential bad programming practice. My gut says he's wrong, but I'm struggling to come up with a good example of where they're a good, legitimate solution. For example, I might be tempted to put the protected m...
Switch(some case) { case 1: // compute something ... return something; break; case 2: // compute something ... return something; break; /* some more cases ... */ case X: // compute something ... return something; break; de...
I'm reading Robert Martin's book "Clean Code" and most of what I've read makes sense and I'm trying to apply as much as I possibly can. One of the simplest most basic things he talks about is that methods should be small, he goes all the way as to say that they should not contain more than three lines. Considering that I broke down a re...
In general, what is the maximum number of parameters a class constructor should accept? I'm developing a class that requires a lot of initialization data (currently 10 parameters). However, a constructor with 10 parameters doesn't feel right. That leads me to believe I should create a getter/setter for each piece of data. Unfortunately, ...
I love languages that evaluate a single expression both as a value and as a boolean value. For example A = 1 evaluates to true, and so does 1. If this practice is very common to the developers in my think tank, is it wrong not to refactor out these expressions, assuming no side effects? I have a long standing discussion at work talkin...
In the sense that a code smell is an indicator of a potential need for refactoring are private methods a code smell? I was looking at some of my own code and it dawned on me that many of my public methods don't actually do anything other than consolidate private methods of the same class. Furthermore, none of the private methods relied ...
If you have for example > 5 left joins in a query is that a code smell that there is ... something wrong with your design? you're doing too much in one query? you're database is too normalized? ...
Hi all, I am interested in exploring the idea that the relationship between methods and the member varialbes they use can give a hint to how the class could be broken up into smaller pieces. The idea is that a group of variables will be closely related to one responsibility and should be contained in one class according to SRP. For a t...
One problem that I have frequently run into lately is the problem of my presenter classes growing too large. Usually, I can chop up a regular large class without skipping a beat. But the presenters sometimes are a little more difficult to pare down, without making the code harder to follow. Especially when the page starts filling up...
I recently came upon some code filed with the following } catch (FooException e) { //do something } catch (BarException e) { //do something } catch (Exception e) { throw e; } which is easily re-written as } catch (FooException e) { //do something } catch (BarException e) { //do something } and if (!(flag == false)) which ...
I have the Rails partial which has a few such checks: <%= if local_assigns.has_key?(:show_blabla) && show_blabla == false %> blabla <% end %> I think that sort of hides the real list of parameters which are used by such partial, and hey local_assigns is not in documentation even. What are your thoughts? ...
I'm looking to see how other programmers find anti-patterns, "code-smells", etc. In particular, what things start setting you off when you're looking at code that tells you, something has gone wrong here? I'm not looking for a list of different patterns like this post. I want to know how you've found such things in code bases you've...
It occurs to me (as I'm reviewing some old code) that there may be a standard way of rating code smells and I'm just not familiar with it. To be clear: I'm familiar with a few taxonomies of code smells, what I'm looking for is a standard rating system for code smells. (Where one would be able to fairly objectively state that one piec...
I have some code that I'm trying to rewrite. The code is designed to be "generic", in that it can be used by many different callers that require different "work flows". It goes something like this: string globalA; int globalB; bool globalC; // Lots more globals in various shapes and forms void TheOneAndOnlyMethod(XmlBasedConfig config)...
Say we have the following code function ABC() { if (....) { if (....) { if (....) { ... } } else { ... } } else { ... } } my question is... Is that 3 levels of If/while-blocks already way too much to withstand? if not, how much levels of these statement blocks can you ...
I am surely missing something huge, I've seen this problem several times so I would like to know your opinion on resolving these code-smells. Background is a windows forms app organized using the MVC architecture pattern. Data is organized in a hierarchical way (tree model), with different types of nodes representing different types of...