I have hardly a year's experience in coding. After I started working, most of the time I would be working on someone else's code, either adding new features over the existing ones or modifying the existing features. The guy who has written the actual code doesn't work in my company any more. I am having a hard time understanding his code and doing my tasks. Whenever I tried modifying the code, I have in some way messed with the working features. What all should I keep in mind, while working over someone else's code?
Does the code have unit tests? If not, I strongly suggest you start adding them. This way, you can write new features/bug fixes as failing tests and then modify the code that the test passes. The more of those you build, the more confidence you will have that your added code has not broken something else.
Writing unit tests for code you do not fully understand will help you understand said code. Of course, functional tests should be added if they do not already exist. My impression was that those already did exist form the OP question. If I am wrong on that point, then these functional tests should be your first step.
In addition as you write these tests I'd encourage you to try to write the tests so that they verify business behavior that the method may have tried to accomplish, not code behavior. Also, don't at all assume the business behaviors you see in the code are the correct ones - If you have someone who could tell you what the application should be doing that is in many cases more valuable than what the code might tell you.
In my opinion, the fastest way to learn someone else´s code, (especially when changes triggers unexpected behavior as you describe) is to step through the code using a debugger.
Begin with stepping through what seems to be the main loop / main methods of the program. Use the step into and step out functions to see what different methods does. This will teach you the general structure of the code.
After that, divide and conquer by stepping through and learning the different parts of the program on a deeper level. In most debuggers you can study variables and their current values. Study how they change and when.
Set out breakpoints on methods that trigger behaviors that concerns you. For example if you are trying to change a text in the program, and the text keep changing back to the original value, set breakpoints on all places where the text is changed, or try to move all these changes to one single method. Use the call stack to see from where this method is called, etc etc.
If changing a line of code causes unexpected changes on other places, put a breakpoint on that line, and see what happens there, for example by checking the values of current variables in scope, using step into, or the call stack to see from where the call came.
By doing this alot, you will start to learn the structure of the code surprisingly fast. I started out just like you did on my first programming jobs, with lots of code that had been written many years ago and been changed by many people through many years. The code was not mine only since there where other people working on it at the same time. I couldn't rewrite it all at that point. Writing tests for all that code would have taken me months or years. The debugger really saved me, don't know how I would have learned the code without it...
The first thing to keep in mind is that more time is spent reading code than writing code. Spend the time to understand how the other guy worked -- his style and approach to problems.
Try to adopt the existing style as much as possible -- otherwise the guy after you will have twice as much adjusting to do.
Dealing with someone else's code is the norm, not the exception, you need to become adept at figuring out how the other guy would have solved a problem or implemented a feature. Once you've done that, you will find it easier to deal with his code.
Don't be too quick to assume the other guy's code stinks.
But always be suspicious.
But yeah, it takes time to understand another dev's code. The more a function or object is used by multiple parts of the system the more careful you need to be. If you can solve the problem closer to the symptom, that can sometimes be helpful. For example, normalize incoming data from another object on the problem-object's side of the fence after the data is delivered but before anything else happens.
It is a bad sign when changing one thing breaks another unexpectedly. If you have any other more experienced developers you can rely on for help, I'd recommend getting them to look at stuff that's causing you issues. At the very least you may pick up a few things watching them debug.
In an ideal world, all code written by a given developer will be well documented, well structured and comprehensibly tested, both with automatic tools such as unit tests and use case scripts that a user runs through to check that you get the expected result.
However, the first thing you'll learn is we don't live in an ideal world!
A lot of developers don't document their code properly, if at all, they mix business logic with unrelated code, and the only test they do is a quick run through what they expect to be the normal use case.
When working with code like this, the first thing you have to do is establish what it is meant to do. If there are comments they may give you clues, but don't count on it. It's my experience that many coders aren't good at explaining themselves and even if they do leave comments they might be meaningless. However, unless you're the only coder in the company, someone surely must have at least a basic idea of what the code is for and what it's meant to do. Ask around!
If you have unit tests, then they will make your life a hell of a lot easier. If you don't, then part of learning the codebase may involve writing unit tests for code that already exists. Normally this isn't considered good practice because if you write unit tests to fit the existing code, you'll end up with unit tests that think the code works as is (they'll be written to assume that behaviour that's actually a bug is correct), but at least it gives you a baseline. If you later discover that some behaviour that you thought was correct is in fact wrong, you can change the unit test to test for what the expected result is rather than the result the code gives now. Once you have a unit test, you can make changes and assess what side-effects any changes you make have.
Finally, the best resource you have when dealing with an undocumented piece of code is to ask the end users. They may know nothing about code, but they know what they want the application to do. Requirements gathering is the first stage in any project, and talking with the prospective users of the system to be developed is always an important part of that. Just think of it as doing the requirements capture stage for a new project that just happens to already have been built.
Do bear in mind that even well-written and well-documented code can be difficult for an outsider to understand. Code is essentially an expression of how the person who wrote it was thinking at the time, and everyone has their own unique thought process. You'll have to learn to be a bit patient, and to be a detective. Being able to get into another person's thought process is difficult, but it's an essential skill for a programmer doing maintenance on existing code. As most coding (about 70%) is related to maintaining existing code, it's an important skill to learn.
Oh, and now that you've seen the pain that poorly-documented, untested and jumbled code can cause, you'll not do it to the next poor developer to come along, right? :) Learn from your predecessor's mistakes, comment your code well, make sure that every module has a clearly defined responsibility that it sticks to, and make sure you have a comprehensive set of unit tests that you either write first (for TDD methodologies) or at least alongside the code being developed.
Keep in mind that the ability to read code you haven't written is a very valuable skill, probably more valuable than writing code. Unfortunately, this is widely understated and under-taught at schools.
What I am trying to say is that it's normal that you don't always understand code upon reading it the first time (just like it's normal that you don't write perfect code the first time). If you accept that it takes time to get a foreign code, than you won't mind putting in the extra effort. A small summary:
Unit tests would be ideal, but not always realistic; especially if you work in a large organization with heavy bureaucracy.
Learn to use your Version Control system properly; you'll never break the existing (not really never, but it's a good safety net).
Don't assume it's bad simply because you don't understand it instantly. Don't assume it's good simply because it works. The important thing is that you understand the previous maintainer's style of code and adapt your added lines to his style. The maintainer coming after you will thank you for it.
In some companies, unfortunately, the difficulty of reading code can be underestimated. This is common in large corporations with rigid processeses. They'd often (implicitly) prefer you push code that works quickly rather than take your time to write something clean. I'll leave it up to you to decide where your team stands on this point.
Finally, never forget that reading code is a skill. The more you do it, the better you'll get at it. Another way to say this is that the only way to get good at it is to practice it many times. Like mentionned above, reading code is and will be a much larger part of your job than writing is.
Judging from your problems with inadvertently breaking stuff, I'm going to assume that the code isn't covered by automated tests. Step #0 would be to immediately order and read Working Effectively with Legacy Code by Michael Feathers. It is simply invaluable.
The basic steps I would suggest:
- Cover the code with tests covering the current functionality.
- Refactor until understandable.
- Write a test for the new or modified functionality.
- Implement the new functionality.
- Refactor until satisfaction.
I deliberately leave out specifying the flavor of the tests (unit, integration, ...) - just get some kind of automated test coverage.
(and, yes, follow the coding style in terms of layout and naming)
As mentioned earlier: welcome to the real world. I can only agree with earlier answers. I'll only wish to extend the answer with my work experience about time estimates.
A good suggestion it to make you boss clear, it will take time to learn how the other developer(s) think. Usually you will experience that the current solution often depends on the age and experience of the developer.
If you are lucky, the task at hand has to be analysed and understanding the documentation will help you a lot (but this is often not the case).
My experience is that when modifying others code, try not to change code which does not involve your current task. You may know a better solution or it could be written in a more intuitive way, but changing it often leads to problems like:
- The task will take longer and your boss will not understand it.
- The code you change must be tested, and it costs. The current solution has been tested and approved.
- It will be difficult to see what changes solves the current task, and which are 'just' corrections.
But do not hesitate to tell your boss, if you see something you think should be different (it just shows you can think).
Finally, make sure that you have enough time to make the solution. A faster solution comes with experience. But there is seldom a fast solution, as this is the first/major reason for errors and unmaintainable code.
Think of it like performing an operation on a person.
You look inside for the problem you need to fix and notice that most of the arteries, etc. are not set up the way you would do it - so you cut and chop them around until it looks right to you and then fix the problem.
Amazingly your patient dies almost immediately.
Legacy applications are the same. They already have a way of working - you need to understand the various components in the software and how they relate to each other and then make your change so it works the same way. It is not a exciting as letting your creativity go wild, but you can do that on personal projects.
I would ask a senior engineer to sit down with you for an hour or so every Monday and explain a different aspect of the system. Make notes of what he says and email the notes to him and your manager to see if your manager has anything to add. You should get up to speed quite quickly this way.
As for how to not break things, first of all make sure you understand what the system does. Test before - make your change - test afterwards. There are no magical formulas; as you gain experience you will get better - or get fired I guess!
One thing that I haven't really seen touched on here - don't work on an island.
Unless you are the only programmer at your outfit, there is bound to be somebody that has more experience than you, and quite possibly many people that you can lean on.
Ask questions. Lots of them.
Don't worry about "annoying" somebody else (within reason) - I'd rather somebody interrupted me for a question or two during a normal development cycle, than having to put out a fire in a production environment later on.
When you are ready to check something in, review it with your mentor(s). They should be able to tell you not only if something will break something else, but more importantly, why. Reviewing code will also make the mentor a better programmer, giving him/her a view into the system that they may not otherwise look at as often.
Remember - you are not only learning the system as any new employee would need to do, but you are also learning how to become a programmer.
And five years on, encourage the next New Guy to use you as a mentor.
When it comes to debugging code, remember: there's always a reason. When you've been trying to find and fix the same stupid bug for a few days and you aren't making any progress, it's tempting to start thinking one or more of:
I'm just not smart enough to figure out how this code works
The guy who wrote this code had no idea what he was doing
Magic is involved: very black magic
Those are all forms of giving up. The antidote is to always remember that computers are deterministic: there's always a reason for what they do. The code may smell like low tide at a fish cannery and resemble a giant bowl of linguine, but by being relentlessly rational and keeping an open mind, you'll figure it out.
Whether you write unit tests where possible or write small applications involving the code you are modifying, you will have to look at, understand, and then document the logic.
If the code works mostly -- sounds like it does -- then I would preserve the style of the code formatting for that module, whether it's your style or not. It keeps things uniform. However, good comments never go out of style.
I advise a test system and test platform, where you can modify and test this code, without breaking production.
If you can remove elements of the code into a library, I would do it, unless you are working on a library.
Over time, once you understand the logic, you can rewrite and test.
This advice is gated by the language you're using, the ability to get a test bed, and other constraints you have.
It has been remarked above that you need to understand the purpose of the system, not just the details of the code. A programmer with enough experience to write an order entry system is generally comfortable with the 'moving forward' part, which involves selecting products, formatting an invoice, and processing the payment. Where they get stuck is when the user decides 'no never mind' and starts backing out transactions, or when they make an error in the payment processing and hit the 'back' button. At that point a lot of programmers get flummoxed, because they see code 'showing up out of nowhere' and can't figure out why it's there.
In short, you have to understand not just the 'normal flow', but all the backtracking that's necessary if someone makes a mistake or changes their mind. This gets even worse with supervisor overrides, where some code can only be run with certain account privileges.
If someone writes 10,000 lines of code a year, and an application has a ten year 'life', then a programmer responsible for picking up someone else's work might have to understand 100,000 lines of code. Divide this by 50 lines per page is 2000 pages. If the program is written to a design pattern, the programmer will find that the understanding of one 'block' leads to at least a general understanding of most of the rest. If not, then it's necessary to read every last damn line.
Some programmers 'just do what they're told' and write spaghetti. They never do understand the 'big picture' - they simply put in fixes when users complain. In such a circumstance it is a good idea to start migrating whatever you can to an appropriate pattern. Eventually this may mean recoding stuff that isn't 'broken'. Don't worry about it, just make sure that as long as it's your project it's progressively more maintainable.
There are some really nice answers here. But I think it may also be worth mentioning how much familiarity with good design patterns can help, to read (well-written) existing code and to write readable code.
Of course it can be very confusing when you encounter a
SomethingFactory in existing code, that does not actually follow the factory pattern. But as much as your team and framework allows, it may be beneficial to keep such cases to a minimum.
Following design patterns (where the business needs allow it) can also significantly reduce code duplication which in turn reduces bugs on future modifications.
Some good sources on design patterns are
and of course the book
Tracing the flow of control between methods is very important in developing a mental map of the business logic.
Our solution is based on the acknowledgment that one of the few trustable piece of information available when approaching a legacy system is the running system itself. Our approach reifies the execution traces and uses logic programming to express tests on them.
Generating a workflow data model is the best way to analyze all the code paths:
In practice, code review becomes unwieldy in legacy scientific workflows. The origin of such workflows means that software engineering practices may not have been applied during development, leading to a code base which is effectively obfuscated. While static analysis has well developed techniques for dataflow analysis, they fail to support behavior in real world workflows. For example, when a configuration file must be loaded to determine how data should be processed, or when dynamic code evaluation is used.
And visualizing the workflow is ideal:
A common motif, lending itself to visual development, is the presentation of a workflow as a graph. This shields the user from the underlying complexity of resources and execution
- Understanding Legacy Workflows through Runtime Analysis (pdf)
- Object-Oriented Legacy System Trace-based Logic Testing (pdf)
- console.trace() - Web APIs | MDN
- Tracing method calls via Proxies
- How To Spy on Your Ruby Methods
- Easy tracing of nested function calls in Python - Eli Bendersky's website
- Method entry/exit logging – Mebyon Kernow
- Trace: Instrumentation to Show Function Calls
- -Xtrace -
- GitHub - johnno1962/SwiftTrace: Trace Swift and Objective-C method invocations
- Debug::Trace - Perl extension to trace subroutine calls - metacpan.org