Posts Tagged ‘code-quality’
Programmers have to work with large amounts of bad code. Sometimes its your own code, but other times its another programmer’s code. If you have to frequently work with another programmer’s bad code, you will eventually wish to approach them about their code quality, however how to do this without causing offense or resentment is not always easy. This article is about the approach I would take when wanting to casually talk with another programmer about their code quality.
Determine the problem
First off, determine what exactly the problem is, and verify that it is worth bringing up. Ask yourself:
- Do you work with this person on a regular basis?
- Will you be working with, and maintaining their code in the future?
- Is this an actual problem that harms productivity, performance, or security, or is it simply a case that your preferences are different from theirs? For example, simply complaining that “your naming convention doesn’t match mine” is not a very good reason, however saying “your naming convention makes it hard to understand what is going on in the code” is a much better.
If you answered yes to all of those, then it sounds like you might have a good reason to approach someone about their code quality.
Gather your arguments
Once you’ve determined there is a valid reason for trying to change the way another programmer does things, get your arguments prepared. Identify why the code is harmful to the development environment, and figure out the ideal way of coding it instead. Pick some code samples illustrating the problem, and write your own way of coding the same piece. Determine what your arguments are for why your code is better than the existing code sample. Don’t use arguments such as “it’s best practice”, but instead explain why it’s a best practice (readability, maintainability, reusability, etc)
Approaching the other programmer
Now that you know your arguments for why you think your code is better than theirs, and have a clear idea of what the ideal code should look like, arrange a time to talk with the other programmer.
This could be a casual conversation, or it could be setting up an appointment to talk with them about your concerns. How you choose to approach your co-worker is based on your level of comfort with them, the organization’s environment, and both of your ranks within the organization. For example, if I wanted to talk to the programmer next to me, I might simply ask them if I could have a moment of time whenever they’re available to talk with me about a piece of code, however if it’s my boss, I might send him/her an email asking if I could setup a brief meeting with them to discuss some code I’ve been working on.
Some people might argue that approaching a boss is different than approaching a co-worker, however I feel they are both people and both deserve equal respect. The only real difference I see is that your boss’s time is usually more valuable to the company, so be aware of that when talking with him/her.
What to say
When you actually talk with the other programmer, I usually find a good way to start the conversation is to ask them to explain their code. Ask if there was a reason for their style of coding, or tell them you’ve never seen something coded that way, and ask them why they chose that method. Really listen to what they have to say during their explanation. It could be that their way is right, and you are wrong, in which case take notes and learn from it!
If you still think the code is bad and worth changing, show the other programmer how you would code the same piece of code, and tell them why you would code it that way over his/her way (better performance, fewer chances of errors, easier for other programmers to read/maintain, etc).
Focus on why your method of coding is better, and not why their method is worse. Nobody likes being accused of writing bad code, although in reality we were all new once and I’m sure we’ve all written some horrendous code at some point in our lifetime. Be sure to keep the conversation focused on the code, and avoid turning it personal by talking about the person who wrote the code. It is always best if you can show that you understand that sometimes bad code gets written, whether it was due to time constraints, inexperience, or simply a bad day.
In addition, don’t try and get them to change their old code (unless you are their boss and want them to spend time doing so). Your goal is to educate them so they stop making the same mistake in the future, not berate them over past mistakes and punish them by extra work.
Afterwards, see if he/she still supports their coding style over yours. If they are open to improvement, they will likely change their way of coding going forwards. But if they still prefer to use their coding style, you are not likely to change their opinion, so drop the subject and don’t bring it up again unless their code is actually harmful.
Remember, nobody can write perfect code, and good programmers are always looking for improvement. The world of programming is so big that its impossible to know everything. It’s very likely that the other programmer doesn’t know there is a problem with their code, and would appreciate the input, providing you do it in such a way that you are teaching them, and not insulting them.
So don’t be afraid to approach another programmer about their code quality. I know I would appreciate the conversation, and I think you would too.
A quick search for the words “bad code” and “code smell” on Programmers Stack Exchange fetches a lot of results. So this means that several people are having this problem. This article present a few ways to work with this problem from a non-technical 30000 feet perspective.
A few caveats
- Nobody can write perfect software; you can improve it forever. There is always room for improvement.
- Programs keep changing based on requirements and requirements based on market environment. Hence code is dynamic and prone to error.
- Bad code is not directly proportional to programmer inefficiency. There are a lot of other factors that contribute to bad code.
- This article is written from the perspective of enterprises engaged in the business of software.
- In my opinion, there are a few hopeless cases where you just cannot tame it. A list of them is given at the end of the article.
What is bad code?
Though difficult to define, for the purpose of this article, we can define bad code as some piece of code that is: incomprehensible by reasonable standards,1 lacks in conceptual integrity, doesn’t have a definite structure and is a sign of more bigger problems. Technically, code smell.2
If a bad piece of code is not found, it can quickly turn into a code smell and transform into a black hole that could suck every good part of the program in.
The following are a few explicit instances of bad code
- the person who wrote or maintains the code cannot explain it in a logical manner
- changing one part of a program breaks another
- improper documentation
- lack of conceptual integrity
- lack of standard conventions
- lots of duplicated code
- too many components to integrate and too many layers
Causes for bad code
The causes for bad code are many and the reasons are also multidimensional. So, causes may be viewed from three main perspectives:
- Improper communication3
- Lack of domain knowledge4
- Lack of experience and understanding
- Lack of experience in a particular tool-set or methodology
- Over-engineered code
- Interface kludge
- Lot of individual components to integrate
- Improper layer/tier separation
- Improper selection of technologies
- Anemic models
- Outdated methodologies
- Outdated tool-sets
- Usage of legacy systems
- Too much vendor lock-in
- Wrong hiring policies
There are several other ways that bad code can be made, and each topic in this list deserves a separate article on its own. The following Wikipedia page provides some excellent links.
Ensuring the code is indeed bad
Before embarking on this journey, make sure that the code is indeed bad and it needs to be taken up. The following guidelines would help.
- Do plenty of research and get to the root cause.
- Analyze the code from multiple perspectives. Review it with another person.
- See old versions of the code to know its evolution.
- Always use objective standards.
- Discard petty issues, obvious errors that could be easily corrected.
- If possible, correlate between the code and the bugs created by the code.
- Use standard code coverage tools to analyze the program.
Your Rank in the organization
A program is just not an entire technical pursuit. There are different priorities for the same program at different ranks and the same technique wouldn’t be a good fit for all.5 Thus your rank in the organization gives you the ability to leverage your resources purposefully. The higher the rank, the higher the resources and the bigger your influence albeit with higher responsibility. Enterprise hierarchy is one big tree you can’t always traverse successfully.
For the sake of simplicity, we assume three ranks in the hierarchy bottom-up.
- Rank 1 – people who actually write code – programmers, developers, software engineers – scope limited to a module or a set of modules.
- Rank 2 – people responsible for a project – Project leaders, senior developers, team leaders, software architects – scope covers the entire project in hand.
- Rank 3 – people responsible for the delivery of the software to the customer and revenue to the organization – Project managers, Product managers, delivery managers – scope extends to the successful delivery of the project.
There is a correlation, though not directly proportional, between rank and the nature of problem. Most programmer-centric problems can be attributed to Rank 1, design-centric to Rank 2 and organization-centric to Rank 3.
A few suggestions to approach the problem:
- Know your position and rank in the organization and approach problems that could be realistically solved by you. You may easily talk to a co-worker about refactoring but you can’t expect to implement it across teams if there are tight deadlines. Be pragmatic as what you could solve.
- Talk to the right person. Know the scope and rank of the person you are talking to. You can’t discuss design or methodology change with a Rank1 programmer nor you could be talking variable naming conventions with your boss.
- Take time off. A prior appointment would be good. Never ever discuss in a urgent, haste manner. Proceed on a case-by-case basis. Remember you are pitting for a future change and people doesn’t like change.
- Always give time for reasoning and listen to the person you are talking to and never run into an argument on his style of programming.6
- Despite your best efforts, if your underlying assumption about the code turns to be wrong, graciously accept it.
The following additional guidelines would help for different ranks
Rank 1 Communication
A person in this rank may not be fully aware of the entire software life-cycle. So you are limited about what things you could talk to but this is the rank where most of your suggestions would be accepted. Communication is the X-Factor at this rank.
- Know the qualification, domain experience, expertise and the programming paradigm of the person. Paradigms (Object Oriented,Functional) and domain experience play a huge role in programming style and understanding. Ascertain the right area of concern.
- Ask him to explain the code to you and share your thoughts about it. Show how your proposed method could be better. Use an objective method to stress the advantages arising out of the new method versus its disadvantages.
- Put focus on ease of use, easy maintenance, long-term development of quality software and stress the importance of cascading effects
- Don’t talk things beyond his reach. He is just hired to do programming not to run the organization.
Rank 2 Communication
Persons in this rank do have a lot of technical knowledge so you got to be spot on with your reasoning and facts. Also, keep all your discussions at the conceptual level. Changes at this level may take time to get implemented. Clarity is the X-Factor at this rank.
- Ask the reasons for choosing the specific design or architecture; the constraints, advantages and trade-offs involved.
- If the design is flawed, pinpoint it accurately. If it is a known anti-pattern, name it precisely. If there is a flaw in the business logic, describe it with the industry standards.
- If the design, though technically sound, is too esoteric, try explaining how making it a bit simple would make it more easy.
- Building a good design takes considerable time and energy. So try concentrating on improving the existing design to rectify the flaws rather than proposing a radically new design.
Rank 3 Communication
You are operating at a much higher level where revenue is the main priority. You must prove decisively that the present polices inadvertently lead to programmer inefficiency and thereby increasing cost. So, a lot of work needs to be put in. Remember, you are pitching for a future organizational change. Many a time, you may just not get over the line in this rank, since a lot of other factors are involved in decision-making but if you believe you could make a change, you must give a honest try. There is basically no X-Factor here though some knowledge of political science would definitely help.
- Talk to your peers as how they feel and think about the present scenario and garner valuable support.
- Do organizational research. Read the policies and procedures of the organization and compare them with the competitors.
- Fix a prior appointment and have a formal agenda.
- Never ever use subjective standards.
- Support your views with research papers and authenticated articles.
- Effectively use metaphors.
- Prepare a neat presentation. Use some business buzzwords in it. Do not make it too technical. Nitty-gritty technical points can be taken up at a separate discussion.
- Always present your views with a cost-revenue analysis and show a comparative study with your competitor.
To illustrate, if you are sure that the waterfall methodology of your company is creating delay in software delivery and thereby introducing more bugs due to cramped schedules, try doing the following.
- Select a couple of completed projects.
- Collect the following information regarding the projects.
- Estimated schedules, proposed delivery, actual delivery, estimated budget and final cost
- Number of man-hours spent
- Number of changes proposed by the client and the hours required to complete them
- Bug reports and the time taken to complete them.
- Installation, maintenance and service costs.
- Prepare an alternative schedule in an another methodology (say agile). Be conservative in your approach.
- Now, estimate the hours that would have been saved by the new methodology.
- Include the additional costs that would be incurred
- Calculate the opportunity cost
- Now put these things together and create a nice presentation. Quantify everything and convert it into costs or revenue such as “the new method would lead to a 10% decrease in man-hours that would cut project costs by 7%”
- Initiate for a pilot project so that you could try the new methodology. You are running a big risk here but its worth taking.
A few hopeless (difficult to change) situations in my opinion:
- Cramped schedules.
- Programming thought as a linear function by the management and work allotted on the basis of pure division of labor.
- No upfront design. Starting to code straight away.
- Horses for courses policy. Hiring somebody to do the job.
- Egocentric persons with lethargic attitude.
- Right persons in the wrong jobs and wrong persons in the right job. DBA’s pushed into web interface programming and business analysts doing hands-on programming.
- No clarity in rules of procedures.
- Too much implicit communication.
- Thinking too much.
There are chances that despite your best efforts, your suggestions may not be accepted. Always try to the best of your efforts and if it fails, just adopt an another method. Remember the Pareto principle; You might spend 20% discovering bad code and 80% try correcting it. Also, communicating bad code is 20% technology and 80% psychology.
Our problems are man-made, therefore they may be solved by man. And man can be as big as he wants. No problem of human destiny is beyond human beings.
- John F. Kennedy, speech at The American University, Washington, D.C., June 10, 1963
- The Anti-patterns page at Sourcemarking
- Developer Survival Guide to know a few things other than technical
- Code Simplicity for a few ideas on how things could get complex
- A taxonomy for bad code smells by Mäntylä, M. V. and Lassenius, C
- The Bad Spotters Guide by Diomidis Spinellis
- Why People Speak Bad Code at Kilterish’s Blog
- A few Code Smells
- Design patterns in Dynamic Languages by Peter Norving
Reasonable standards mean the program must be understood by a programmer with the same expertise as the person who wrote it in the first place ↩
Communication here means the ability to correctly understand the business requirements of the client and to explain it to fellow programmers. ↩
Domain here refers to specialization. Domain may be computer-related such as systems programming, web programming, GUI application or business-related such as finance, human resource, production. ↩
The only way to get the best of an argument is to avoid it from “How to win friends and influence People” by “Dale Carnegie” ↩