Let me start by saying that I do not advocate this approach, but I saw it recently and I was wondering if there was a name for it I could use to point the guilty party to. So here goes. Now you have a method, and you want to return a value. You also want to return an error code. Of course, exceptions are a much better choice, but for...
OK, so I know what a code smell is, and the Wikipedia Article is pretty clear in its definition: In computer programming, code smell is any symptom in the source code of a computer program that indicates something may be wrong. It generally indicates that the code should be refactored or the overall design should be reexa...
There's something very unsatisfactory about this code: /* Given a command string in which the first 8 characters are the command name padded on the right with whitespace, construct the appropriate kind of Command object. */ public class CommandFactory { public Command getCommand(String cmd) { cmdName = cmd.subString(0,8)....
So, we saw the post on Code Smells and how to fix them...but what about for us NON object oriented folk? I use embedded C a LOT and while all the responses to the said post were great, I wondered if anyone had some input about the straight-C world? Some of the things mentioned in the other post do not apply (and some were even complete...
I just found myself creating a class called "InstructionBuilderFactoryMapFactory". That's 4 "pattern suffixes" on one class. It immediately reminded me of this: http://www.jroller.com/landers/entry/the_design_pattern_facade_pattern Is this a design smell? Should I impose a limit on this number? I know some programmers have similar...
List examples of programmers doing stupid things in their code. I just came across this little baby in PHP: if(strpos($row['themename'],"some string") !== false){ This is wrong on so many levels. First, there is the double negation in saying 'not equals false'. Then there's the explicit checking of truth (why not do ($something == f...
Routines can have parameters, that's no news. You can define as many parameters as you may need, but too many of them will make your routine difficult to understand and maintain. Of course, you could use a structured variable as a workaround: putting all those variables in a single struct and passing it to the routine. In fact, using st...
In our company, developers always have dozens of important tasks assigned and tight deadlines in which to complete them. In this environment, a code review often shows that their program will execute correctly but is "smelly", i.e. hard to read, hard to maintain and a potential breeding ground for bugs. How do you persuade developers (...
(Background: from a previous job, a co-worker and I would end up discussing the bug pile during lunch. We began to develop a topic called "bug of the week". I doubt I have material for 52 posts a year, but here's the first one...) Reported by: QA tester was reading HTML/JS code to write a functional test of a web form, and saw: if (fo...
Working on a project where a sequential set of methods must be run every x seconds. Right now I have the methods contained within another "parent method", and just sequentially call them right after another. class DoTheseThings() { DoThis(); NowDoThat(); NowDoThis(); MoreWork(); AndImSpent(); } Each method must ru...
I have an application with my object types that inherit from a base class that contains the majority of properties for the application objects. All the object types are stored in one table in the database. The "ClassType" column determines what object type I cast the SqlDataReader row to. Here is my current implementation: SqlDataR...
I have been asked to present examples of code issues that were found during a code review. My audience is mostly non-technical and I want to try to express the issues in such a way that I convey the importance of "good code" versus "bad code". But as I review my presentation it seems to me I've glossed over the reasons why it is impor...
I'm faced with a design decision that doesn't smell to me, but gives me pause. Take a look at the following code sample: public interface IGenerator { ///<summary> /// Combines two generators; performs magic as well /// </summary> BaseGenerator Combine(BaseGenerator next); ///<summary> /// Does what a generator does. ///...
What is the most evil or dangerous code fragment you have ever seen in a production environment at a company? I've never encountered production code that I would consider to be deliberately malicious and evil, so I'm quite curious to see what others have found. The most dangerous code I have ever seen was a stored procedure two linked-...
I just wrote this method: private String getNameOfFileFrom(String path) { int indexOfLastSeparator = path.lastIndexOf('/'); if (indexOfLastSeparator > -1) { return path.substring(indexOfLastSeparator + 1); } else { return path; } } The line that bothers me is: return path.substring(indexOfLastSeparator + 1); Is that a ba...
Yesterday i was playing with jQGrid plugin and ASP.NET. Everything turned out well, my grid is working now, but i have two methods, that make my code smell. smelly methods: private IOrderedEnumerable<Employee> GetOrderedEmployees(Column sortColumn, bool ascending) { switch (sortColumn) { case Column.Nam...
I have been thinking about the Feature Envy smell lately. Suppose I have an object called DomainObject, that responds to a message "exportTo:someExport". This is basically the DomainObject's way to provide a copy of its internal state: exportTo:someExport someExport setX:myX. someExport setY:myY. someExport setZ:myZ. That...
In your experience, what are some "smelly" keywords in class or function names that might be warning signs of bad object-oriented design? I've found that classes containing the word Manager or Base are often suspect. For example, a FooManger often indicates poor encapsulation or a singleton design that is difficult to reuse. A FooBase ...
Excessive use of magic numbers or string literals in code is something of a code smell; not necessarily wrong but worth considering carefully. However, one can set up your editor/IDE to highlight string and number literals in garish colours like pink on lime green. That way, you can't help but notice where you are using these types and...
For example, we have some CSS rules to define our form layout. We use the following markup: <div class="foo"> <label class="bar req">Name<em>*</em></label> <span> <asp:TextBox runat="server"/> <label>First</label> </span> <span> <asp:TextBox runat="server"/> <label>Last</label> </sp...