Working Effectively with Legacy Code
About This Book
Is your code easy to change? Can you get nearly instantaneous feedback when you do change it? Do you understand it? If the answer to any of these questions is no, you have legacy code, and it is draining time and money away from your development efforts.
In this book, Michael Feathers offers start-to-finish strategies for working more effectively with large, untested legacy code bases. This book draws on material Michael created for his renowned Object Mentor seminars: techniques Michael has used in mentoring to help hundreds of developers, technical managers, and testers bring their legacy systems under control.
The topics covered include
- Understanding the mechanics of software change: adding features, fixing bugs, improving design, optimizing performance
- Getting legacy code into a test harness
- Writing tests that protect you against introducing new problems
- Techniques that can be used with any language or platform—with examples in Java, C++, C, and C#
- Accurately identifying where code changes need to be made
- Coping with legacy systems that aren't object-oriented
- Handling applications that don't seem to have any structure
This book also includes a catalog of twenty-four dependency-breaking techniques that help you work with program elements in isolation and make safer changes.
https://www.amazon.com/Working-Effectively-Legacy-Michael-Fe...
You might also want to read Refactoring to Patterns, but the legacy book is more important to start with.
If it has no tests, then I would slowly try to build tests to document the functionality that I need. In your case being Angular that might be having simple html pages with the smallest module that you need.
How to find things? If you're on Windows try AstroGrep https://www.amazon.co.uk/Working-Effectively-Legacy-Michael-...
Lastly, I would raise this because the company might not be aware they are buying a low quality framework that maybe ticks all the boxes in the contract but is in effect impossible to use by their current developers (you), it might be there's other people with more experience in said niche that might be able to help. In the private community maybe some people would be able to accept a short contract to help train you.
Yes. But it’s a great paying skill. There are techniques for working with legacy code.
This is the go to book for it. https://www.amazon.com/Working-Effectively-Legacy-Michael-Feathers/dp/0131177052
I think it would be a good exercise to learn something new and then go back to your old projects and apply them. For example, learn about testing and go back to your calculator and try to get a good chunk of it under tests (another good resource is Working Effectively with Legacy Code, which covers getting projects under tests when they don't have any). Then learn about CI/CD, which use tests, and make a build pipeline for your calculator. Then read Clean Code, read about architectures and patterns, and see if you can improve your architecture. This is just an example, but any of those could give you good talking points if interviewers ask about your projects.
Michael C. Feathers (who wrote the book Working Effectively With Legacy Code) defines legacy code as code without tests. If code is tested, you can modify or replace it with confidence and without tests, you can have no confidence that changes to the code won't break something. If you can't change your code without fear of it breaking, your code is legacy.
What you should have initially are integration tests. These will test that the functions perform as expected and you could hit the actual database for this.
Once you have that savety net you could start refactoring the code to be more maintainable and introducing unit tests.
As mentioned by serbrech Workign Effectively with Legacy code will help you to no end, I would strongly advise reading it even for greenfield projects.
http://www.amazon.com/Working-Effectively-Legacy-Michael-Feathers/dp/0131177052
The main question I would ask is how often does the code change? If it is infrequent is it really worth the effort trying to introduce unit tests, if it is changed frequently then I would definatly consider cleaning it up a bit.
As far as I know, there are no tools for automatically bringing existing code under unit tests - if it were that easy, there should be no new bugs at all, right? As arne says in his answer, if code was not designed to be tested in the first place, it usually has to be changed to be testable.
The best you can do in my opinion is to learn some techniques of how to introduce unit tests with relatively few changes (so that you can introduce the unit tests before you start the "real" modifications); one book on this subject I've read recently is Michael Feathers' "Working Effectively with Legacy Code" (Amazon Link: http://www.amazon.com/Working-Effectively-Legacy-Michael-Feathers/dp/0131177052). Although it has some shortcomings, it has pretty detailed descriptions of techniques how you can easily introduce unit tests.
I've had this problem before and I've asked around (before the days of stack overflow) and this book has always been recommended to me. http://www.amazon.com/Working-Effectively-Legacy-Michael-Feathers/dp/0131177052
This question came up recently on the RSpec mailing list, and the advice we generally gave was:
You may find that the design of code which wasn't driven out by code examples or unit tests makes it awkward to write tests or specs for. This is perhaps what your friend was alluding to. You will almost certainly need to learn a few key refactoring techniques to break up dependencies so that you can exercise each class in isolation from your specs. Michael Feathers' excellent book, Working Effectively With Legacy Code has some great material to help you learn this delicate skill.
I'd also encourage you to use the built-in spec:rcov rake task to generate code coverage stats. It's extremely rewarding to watch these numbers go up as you start to get your codebase under test.
This is often referred to an 'sensing' within a class under test: you have to find a way to tell whether a method has been called. These kinds of things are covered well in the book 'Working Effectively with Legacy Code' which everyone should have (disclaimer: I have no affiliation with the book's author, I just think it's a good book to have).
What does the
CreateRegisteringMailMessage
method do? Does it affect the content of theRegisterModel
instance passed in during the call?Answer to most of your questions http://www.amazon.com/Working-Effectively-Legacy-Michael-Feathers/dp/0131177052
[0]: https://www.amazon.com/Working-Effectively-Legacy-Michael-Fe...
https://www.amazon.com/Working-Effectively-Legacy-Michael-Fe...
https://www.amazon.com/Working-Effectively-Legacy-Michael-Fe...
If you start from scratch you may bump into the same edge cases that the original writers bumped into, and end up with a code that is not much better than the original - even in the original is 2 years out of date.
I’m sure there were cases when writing from scratch was a good call, but I don’t remember hearing about it.
I believe that the mocking support in CppUTest is invasive, i.e. you need to add mocking support to your production code as well. Example:
For non-invasive unit testing of plain C code you could instead use e.g. preprocessor or link seams. Check out Michael Feathers' great book Working Effectively with Legacy Code for details.
Actually, an extract from that book covering those seam types can be found here. I'd still recommend any C programmer to read that book - it's invaluable.
Your
ClassToMock
tightly coupled withFileReader
, that's why you are not able to test/mock it. Instead of using tool to hack the byte code so you can mock it. I would suggest you do some simple refactorings to break the dependency.Step 1. Encapsulate Global References
This technique is also introduced in Michael Feathers's wonderful book : Working Effectively with Legacy Code.
The title pretty much self explained. Instead of directly reference a global variable, you encapsulate it inside a method.
In your case,
ClassToMock
can be refactored into this :then you can easily using Mockito to mockgetMemberOne()
.UPDATED Old Step 1 cannot guarantee
Mockito
mock safely, ifFileReader.readMemeber1()
throw exception, then the test will failled miserably. So I suggest add another step to work around it.Step 1.5. add Setter and Lazy Getter
Since the problem is
FileReader.readMember1()
will be invoked as soon asClassToMock
is loaded. We have to delay it. So we make the getter callFileReader.readMember1()
lazily, and open a setter.Now, you should able to make a fake
ClassToMock
even withoutMockito
. However, this should not be the final state of your code, once you have your test ready, you should continue to Step 2.Step 2. Dependence Injection
Once you have your test ready, you should refactor it further more. Now Instead of reading the
MEMBER_1
by itself. This class should receive theMEMBER_1
from outside world instead. You can either use a setter or constructor to receive it. Below is the code that use setter.These two step refactorings are really easy to do, and you can do it even without test at hand. If the code is not that complex, you can just do step 2. Then you can easily test
ClassToTest
UPDATE 12/8 : answer the comment
See my another answer in this questions.
UPDATE 12/8 : answer the comment
It depends.
There are something you might want to think about before you do a massive refactor like that.
If I move
FileReader
outside, do I have a suitable class which can read from file and provide the result to every single class that needs them ?Beside making classes easier to test, do I gain any other benefit ?
Do I have time ?
If any of the answers is "NO", then you should better not to.
However, we can still break the dependency between all the classes and
FileReader
with minimal changes.From your question and comment, I assume your system using
FileReader
as a global reference for reading stuff from a properties file, then provide it to rest of the system.This technique is also introduced in Michael Feathers's wonderful book : Working Effectively with Legacy Code, again.
Step 1. Delegate
FileReader
static methods to instance.Change
To
By doing this, static methods in
FileReader
now have no knowledge about how togetMemberOne()
Step 2. Extract Interface from
FileReader
We extract all the method to
AppProperties
, and static instance inFileReader
now usingAppProperties
.Step 3. Static setter
We opened a seam in FileReader. By doing this, we can set change underlying instance in
FileReader
and it would never notice.Step 4. Clean up
Now
FileReader
have two responsibilities. One is read files and provide result, another one is provide a global reference for system.We can separate them and give them a good naming. Here is the result :
END.
After this refactoring, whenever you want to test. You can set a mock
AppProperties
toGlobalAppProperties
I think this refactoring would be better if all you want to do is break the same global dependency in many classes.
Example from Book: Working Effectively with Legacy Code
To run code containing singletons in a test harness, we have to relax the singleton property. Here’s how we do it. The first step is to add a new static method to the singleton class. The method allows us to replace the static instance in the singleton. We’ll call it setTestingInstance.
Now that we have that setter, we can create a testing instance of a PermitRepository and set it. We’d like to write code like this in our test setup: