Working on someone else's code

Question

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?

Welcome to the real world where code lives forever and programmers come and go. — , May 23 '12 at 06:19
you will have to invest time and effort in understanding what the code does and what its intended to do. This i think is the golden guideline which generally all follow. — V4Vendetta, May 23 '12 at 06:25
@gnat I don't think we know whether the existing code is structurally unsound - 1 years experience isn't a lot and an "inexperienced" coder jumping into an experienced coders work could be the problem here — rickyduck, May 23 '12 at 09:38
@rickyduck **"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."** = existing code is structurally unsound, at least for the OP - which is essentially all that matters — gnat, May 23 '12 at 09:49
@Buhb Even I am thinking that its my code when I am working and what makes me think that the code doesn't belong to me is the fact that, when I try to modify one feature, it tends to affect a feature else where. So technically I am responsible for this mess. I am not that experienced to say that the previous programmer's work is good or bad. But I am not expecting any other feature to change, when I am working on modifying a particular feature. As a starter/rookie developer, this is where I am experiencing the most difficulty and I am trying different ways to tackle that. — Xavi Valero, May 23 '12 at 09:51
@gnat again that could be merely down to OPs inexperience and lack of knowledge. if I went into a colleague's function, deleted a line of essential code, put said code live and broke it, that's down to my negligence, not structurally unsound code — rickyduck, May 23 '12 at 10:00
@Buhb: But, 6 months from now, when you come back to it, it will be someone else's code, even the parts you wrote ;-) — Jörg W Mittag, May 23 '12 at 10:20
Be happy. You are developing a critical skill that will differentiate you from people with less experience or academic-only experience. It's *supposed* to be hard. That's why it's valuable. — Scott C Wilson, May 23 '12 at 11:50
In my similar situation, I find it amusing that each programmer leaves comments like 'whhyyy bobb whhhyy!' — Jeff, May 23 '12 at 12:43
@rickyduck to fully rely on a skill level of a developer is first of all a problem of _project_. I recently have been handed over a project from my friend and colleague who hand-picked me to maintain after his leave. I can call him anytime for advice, his code is good and I am well used and like his style - but every day I thank God that QA guys cover my back in working with it. — gnat, May 23 '12 at 13:36
@gnat for sure, I totally agree, however (no offence to OP) this company does not sound like the type that would have a QA team, let alone colleagues hand picking predecessors, as it has relatively inexperienced programmers working on code that an experienced programmer authored, with no guidance or info, let alone experience. (I think we can both agree this company needs to rethink its process of editing and updating code) — rickyduck, May 23 '12 at 14:43
I would also recommend reading this book. http://pragprog.com/book/pad/practices-of-an-agile-developer. It has tons of material about coding, collaborating with other programmers and reading other peoples code. — pkurek, May 23 '12 at 13:50
Voting to reopen, even if the code in question is unsound, the QUESTION is about code, good or bad, while the "duplicate" is limited to unsound code. The answers are thus different. — jmoreno, Jan 17 '13 at 03:32
"It's not someone else's code. It's your code now.": See http://www.paulgraham.com/head.html, point 7. — Giorgio, Jan 17 '13 at 09:41

score 59 · Answer 1 · edited Apr 12 '17 at 07:31

59

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.

Eagle76dk makes a great point about getting your manager on board for doing this work -- more details in Eagle76dk's post.

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.

edited Apr 12 '17 at 07:31

Community

1

answered May 23 '12 at 06:35

Sardathrion - against SE abuse

3,706
3
19
34

12

Writing unit tests can be easier said than done though, depending on the code and its dependencies... – Svish May 23 '12 at 09:25
1

@Svish: Good point. I never implied it would be easy, just that it is worth doing even if some refactoring is needed to make the code more suitable for unit testing. – Sardathrion - against SE abuse May 23 '12 at 10:07
46

Writing unit tests on existing code is a very challenging and time consuming task if the code was not designed for it. To do it on code which you do not understand, for a beginner, may be a very heavy work that will never end. It would be a way to start learning the code, but I wouldn't say that it's the obvious way to start learning the code. – jake_hetfield May 23 '12 at 10:40
3

Unit tests are best written during development. In case you have to fix a bug and don't understand the design or have no specification you are prone to add unit tests which approve of existing bugs. Sometimes bugs are misfeatures. Because of this I propose to establish **functional tests** instead of unit tests in this case first. That means find example uses which generate the results the user approves. Make test cases by writing these situations, actions and results down thoroughly. If your functional tests cover all user stories and work after your patches, you are fine without unit tests. – Alfe May 23 '12 at 11:41
2

Writing unit tests is the bottom-up approach and will take an immense amount of time and is thus often not pragmatic in large projects. Writing the whole thing anew may be faster in this case. You may well find (and then need time to fix) unit bugs which aren't important because the case never occurs in a functional context. – Alfe May 23 '12 at 11:46
@Alfe: As I said, I assumed that there already exist a suit of functional test cases. That said, the line between a unit and a functional test is not well defined. For example, given that config file parsing is simple and done in one function, would *read a configuration file* be a unit test or a functional test? – Sardathrion - against SE abuse May 23 '12 at 12:10
Reading a config file in itself is not a function of a software product, so I would (I think now ;-) never cosider this a functional test. Functional tests derive from user stories. If actually a user tells the story how he once wanted to read that config (without ever using it), then I'd call it a functional test. – Alfe May 23 '12 at 12:30
this is all fine and good... assuming your code can be unit-tested. I've worked with a codebase that is just a mess of PHP scripts thrown together with no way to unittest them without significant refactoring, but you can't do those significant refactoring confidently since there is no unittest. – Lie Ryan May 23 '12 at 13:07
in fact not having test isn't the worst of its sin, there is no correct indentation whatever tabwidth you're using; mixing business and display logic; using raw sql queries; lots of copy-paste code, often with small but significant differences; absence of classes; no version control; nobody around that understands the code; no documentation; and the list goes on. – Lie Ryan May 23 '12 at 13:22
Writing tests might not be the easiest way to start learning the code, but they're definitely the right place to start _maintaining_ the code. (Getting full test coverage… well, good luck with that!) – Donal Fellows Jan 18 '13 at 22:47
1

As you write these tests I'd strongly 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. – PremiumTier Mar 14 '13 at 23:40
@PremiumTier: Could you merge your comment to the answer? – Sardathrion - against SE abuse Mar 15 '13 at 07:44

score 46 · Answer 2 · edited Oct 27 '13 at 22:34

46

In addition to a another answer which mentioned unit tests, I would suggest that you make sure everything is in version control so you're able to revert your changes easily. And making small changes to make the code more maintainable.

edited Oct 27 '13 at 22:34

Peter Mortensen

1,050
2
12
14

answered May 23 '12 at 06:42

Klee

1,313
7
9

11

Good point indeed but I kinda assumed that anyone now a days do use (read: should use) version control... – Sardathrion - against SE abuse May 23 '12 at 08:39
6

You'd be surprised. I've worked as a contractor at a number of companies where only the final cut of code was committed. Honestly. – immutabl May 23 '12 at 09:15
4

To 5arx's point: If the company culture is only to submit perfect code, one could maintain their own personal Git or Mercurial repository. This is especially easy if the company's "real" version control is SVN. – Dustin Rasener May 23 '12 at 10:57
2

+1 and +1 to 5arx's comment. I've done integration work at REALLY large companies where the version control system consists of writing the date, your name and a comment in the file. After being used with working with git, this seems frighteningly inefficient and bug-prone. – Leo May 23 '12 at 13:01
1

@Sardathrion You know what happens when you "ass u me"... – WernerCD May 23 '12 at 14:19

jake_hetfield · Answer 3 · 2012-05-23T11:03:22.670

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...

I think this is the only realistic answer, writting unit tests for a huge app without them its non practical — CommonSenseCode, Nov 24 '16 at 23:15

score 30 · Answer 4 · answered May 23 '12 at 06:44

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.

Erik Reppen · Answer 5 · 2013-06-17T18:25:12.627

21

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.

edited Jun 17 '13 at 18:25

answered May 23 '12 at 06:47

Erik Reppen

6,243
31
34

9

+1. Resist the temptation to rewrite blocks you don't understand - you will almost certainly introduce new bugs doing this. Instead, move slowly and methodically through the code, only making changes where new functionality (or bug fixes) are actually required. – Scott C Wilson May 23 '12 at 11:52
1

I'd say multiple times a year I misjudge. Just did it today and realized every last of 5 items I thought were problematic were there for a reason. He/they could have left them more clearly marked but I could have wasted less time assuming he/they didn't go there for good reason. – Erik Reppen Jul 23 '14 at 04:52

score 14 · Answer 6 · answered May 23 '12 at 07:04

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.

score 13 · Answer 7 · answered May 23 '12 at 08:24

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.

score 11 · Answer 8 · answered May 23 '12 at 08:53

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)

score 10 · Answer 9 · edited Oct 27 '13 at 22:41

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.

score 5 · Answer 10 · edited Oct 27 '13 at 22:43

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!

score 3 · Answer 11 · answered May 23 '12 at 12:48

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.

score 2 · Answer 12 · answered May 23 '12 at 13:17

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.

score 1 · Answer 13 · answered May 23 '12 at 12:37

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.

"good comments never go out of style"...unless the code changes. While comments can sometimes be helpful, always take them with a bucket of salt - you'll have to verify that the code actually does what the comment says it does. Too often someone will change a line of code but leave in an existing - and now irrelevant - comment. — dj18, May 23 '12 at 12:46
@dj18 Agreed, and cleaning up old comments is part of writing code. I'm saying to keep the format -- if possible -- for uniformity, but commenting is not a bad thing. — octopusgrabbus, May 23 '12 at 12:54

score 1 · Answer 14 · answered May 23 '12 at 14:32

1

Try to use some code analyzer tools to find unused code which can be deleted - so at least you do not have to worry about this code.

answered May 23 '12 at 14:32

FrVaBe

131
4

score 1 · Answer 15 · answered Oct 27 '13 at 20:07

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.

score 0 · Answer 16 · answered Oct 27 '13 at 19:35

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

http://sourcemaking.com/design_patterns

http://www.oodesign.com/

and of course the book

http://www.amazon.com/Design-Patterns-Elements-Reusable-Object-Oriented/dp/0201633612

score 0 · Answer 17 · answered Sep 12 '18 at 18:51

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

References

score -1 · Answer 18 · answered May 23 '12 at 16:20

Make sure you're using a program that helps you finding stuff in the current file. Nothing is worse than knowing the thing you're looking for is in the current file, but you scroll and scroll and can't find it. An outline view in the tool you use to edit the code really helps with that problem.

Working on someone else's code

18 Answers18

Linked