One of the inevitable scenarios we encounter as software engineers is that we have trouble changing or adding a feature to code that we did not create, that we are unfamiliar with, and that has nothing to do with the part of the system we are responsible for. Although this may be a complicated and difficult task, but due to the use of other developers to write code has a lot of flexibility, so we can get big benefits, including increasing the scope of our influence, fix software rot and learning we don’t know before system part (what’s more, also can learn the technology and skills of other programmers).
Given the annoyances and advantages of using code written by other developers, we must be careful not to make some serious mistakes:
- Our sense of self: We may feel like we know best, but usually we don’t. We’re changing code that we know very little about — we don’t know the author’s intentions, the decisions that led to it, the tools and frameworks that the author had at his disposal while writing the code, and so on. Humility is worth everything. You are worth it.
- Self-awareness of the original author: The code we are about to touch was written by another developer, with a different style, constraints, deadlines, and personal life (consuming his or her time outside of work). It’s only when we start to question the decisions he or she has made or why the code is so dirty that the person starts to look at himself or herself and not get cocky. We should make every effort to have the author help us in our work, not hinder us.
- Fear of the unknown: A lot of times, we’re going to be dealing with code that we know little or nothing about. Here’s the scary part: we’ll be responsible for any changes we make, but we’re basically walking around in a dark room with no light. Instead of worrying, we should build a structure that allows us to be comfortable with changes of varying sizes and allows us to make sure we don’t break existing functionality.
Since developers, including ourselves, are people, it’s useful to deal with a lot of human nature when dealing with code written by other developers. In this article, we’ll look at five techniques we can use to make sure we use our understanding of human nature to our advantage, get as much help as possible from existing code and original authors, and make the code written by other developers end up better than the original. While the five methods listed here are not exhaustive, using the techniques below will ensure that when we end up making changes to code written by other developers, we can be confident that our existing features will work and that our new features are compatible with our existing code base.
1. Make sure tests exist
The only way to be really confident that existing functionality in code written by other developers actually works the way it’s supposed to, and that any changes we make to it won’t affect the functionality, is to back up the code with tests. When we come across code written by another developer, the code can be in one of two states :(1) there is not enough testing, or (2) there is enough testing. In the former case, we are responsible for creating tests, while in the latter case, we can use existing tests to ensure that any changes we make don’t break the code and learn as much as we can about the intent of the code.
Creating a new test
This is a sad example: we are responsible for making changes to other developers’ code, but there is no way to ensure that we don’t break anything when we make changes. It’s no use complaining. No matter what condition we find the code in, we still need to touch the code, so if the code breaks, it’s our responsibility. So when we change code, we need to control our behavior. The only way to be sure you’re not breaking code is to write your own tests.
While this is tedious, it allows us to learn by writing tests, which is its main advantage. Suppose the code now works, and we need to write tests so that the expected input leads to the expected output. As we worked through this test, we gradually learned about the intent and functionality of the code. For example, give the following code
Copy the code- public class SuccessfulFilterTest {
- Private static final double threshold_salary = 68330.0;
- @Test
- public void under30AndNettingThresholdEnsureSuccessful() {
- Person person = new Person(29, THRESHOLD_NET_SALARY);
- Assert.assertTrue(new SuccessfulFilter().test(person));
- }
- @Test
- public void exactly30AndNettingThresholdEnsureUnsuccessful() {
- Person person = new Person(30, THRESHOLD_NET_SALARY);
- Assert.assertFalse(new SuccessfulFilter().test(person));
- }
- @Test
- public void under30AndNettingLessThanThresholdEnsureSuccessful() {
- Person person = new Person(29, THRESHOLD_NET_SALARY – 1);
- Assert.assertFalse(new SuccessfulFilter().test(person));
- }
- }
We don’t know much about the intent of the code or why Magic Number is used in the code, but we can create a set of tests where known inputs produce known outputs. For example, by doing some simple math and solving the threshold salary problem that constitutes success, we found that if someone is under the age of 30 and earns approximately $68,330 per year, they are considered successful (according to this code). While we don’t know what those magic numbers are, we do know that they do reduce the initial salary. Thus, the threshold of $68,330 is base salary before deductions. Using this information, we can create some simple tests, such as:
Copy the code- public class SuccessfulFilterTest {
- Private static final double threshold_salary = 68330.0;
- @Test
- public void under30AndNettingThresholdEnsureSuccessful() {
- Person person = new Person(29, THRESHOLD_NET_SALARY);
- Assert.assertTrue(new SuccessfulFilter().test(person));
- }
- @Test
- public void exactly30AndNettingThresholdEnsureUnsuccessful() {
- Person person = new Person(30, THRESHOLD_NET_SALARY);
- Assert.assertFalse(new SuccessfulFilter().test(person));
- }
- @Test
- public void under30AndNettingLessThanThresholdEnsureSuccessful() {
- Person person = new Person(29, THRESHOLD_NET_SALARY – 1);
- Assert.assertFalse(new SuccessfulFilter().test(person));
- }
- }
With these three tests, we now have an idea of how existing code works: if someone is under 30 and earns $68,300 a year, they are considered successful. While we could create more tests to ensure that critical cases (such as blank ages or salaries) function properly, a few short tests not only gave us an idea of the original functionality, but also gave us a set of automated tests that we could use to ensure that we didn’t break existing functionality when we made changes to existing code.
Using existing tests
If we have enough code to test components, we can learn a lot from testing. Just as we create tests, by reading tests, we can understand how the code works at the functional level. In addition, we can see how the original author made the code work. Even if the test was written by someone other than the original author (before we touched it), it still gives us a sense of what other people think of the code.
While existing tests can help, we still need to take this with a grain of salt. It’s hard to say whether the tests are keeping up with the development changes to the code. If so, then this is a good basis for understanding; If not, then we need to be careful not to be misled. For example, if the initial salary threshold was $75,000 per year and later changed to our $68,330, the following outdated test could lead us astray:
Copy the code- @Test
- public void under30AndNettingThresholdEnsureSuccessful() {
- Person person = new Person(29, 75000.0);
- Assert.assertTrue(new SuccessfulFilter().test(person));
- }
The test will still pass, but not as expected. The reason it passes is not because it is exactly the threshold, but because it is beyond the threshold. If this test component includes a test case where the filter returns false when the salary is $1 below the threshold, the second test will fail, indicating that the threshold is wrong. If the suite does not have such tests, stale data can easily mislead us about the true intent of the code. When in doubt, trust the code: As we stated earlier, solving the threshold indicates that the test is not aligned with the actual threshold.
Also, look at the repository logs (that is, Git logs) of your code and test cases: If the code was last updated more recently than the test was last updated (significant changes have been made to the code, such as changing thresholds), the test may be outdated and should be viewed with caution. Note that we should not completely ignore tests, as they may still provide us with some documentation about the intent of the original author (or the developer who recently wrote the test), but they may contain outdated or incorrect data.
2. Talk to the people who wrote the code
In any job that involves more than one person, communication is crucial. Whether it’s a business, a cross-country trip, or a software project, a lack of communication is one of the most effective ways to hurt a mission. Even if we communicate when we create new code, the risk increases when we touch existing code. Because we don’t know much about the existing code at this point, what we do know may be misleading or only represent a fraction of it. To really understand existing code, we need to talk to the people who wrote it.
When we start asking questions, we need to make sure that the questions are specific and aimed at achieving our goal of understanding the code. Such as:
- Where does this snippet fit best on the system?
- Do you have any designs or diagrams?
- What pitfalls should I look out for?
- What does this component or class do?
- Is there anything you wanted to put into the code that you didn’t do at the time? Why is that?
Always be humble and actively seek out the real answer from the original author. Almost every developer has encountered a situation where he or she looks at someone else’s code and asks herself, “Why is he or she doing this? Why don’t they do it?” And then spend hours coming to a conclusion that could have been reached simply by the author’s answer. Most developers are talented programmers, so even if we come across a seemingly bad decision, there’s probably a good reason for it (probably not, but it’s best to study someone else’s code assuming they’re doing it for a reason; If not, we can change it by refactoring).
Communication is a secondary side effect in software development. Conway’s Law, originally formulated by Melvin Conway in 1967, states:
- Design systems for any organization… Will inevitably result in a design that mirrors the organization’s communication structure.
This means that a large, tight-knit team may produce integrated, tightly coupled code, but some smaller teams may produce more independent, loosely coupled code (see Demystifying Conway’s Law for more information on this correlation). For us, this means that our communication structure affects not only specific code segments, but the entire code base as well. Therefore, it is definitely a good idea to communicate closely with the original author, but we should check ourselves not to rely too much on the original author. Not only does this have the potential to annoy the original author, it can also create unintended coupling in our code.
While this helps us delve deeper into the code, it assumes access to the original author. In many cases, the original author may have left the company or happen to be out of the company (e.g. on leave). What should we do in this case? Ask someone who might know something about the code. This person doesn’t have to have actually worked on the code, he or she could have been around when the author wrote the code, or he or she could have known the author. Even a single word from someone close to the original developer can enlighten other unknown snippets of code.
3. Delete all warnings
There is a well-known concept in psychology called The “broken window theory,” which is described in detail by Andrew Hunt and Dave Thomas in The Pragmatic Programmer (pp. 4-6). This theory was first developed by James Q.Wilson and George L. Kelling and is described as follows:
Imagine a building with several broken Windows. If Windows are not fixed, vandals tend to break more Windows. Eventually, they may even break into the building and, if it is unocked-out, occupy it or set fire to it. Consider sidewalks, too. If there is a pile of rubbish on the road, there will be more before long. Eventually, people will even start throwing takeout trash there and even breaking cars.
The theory states that it is human nature to neglect care for an object or thing if no one seems to care for it. For example, if a building already looks messy, it is more likely to be vandalized. In software, the theory means that if a developer finds the code is already a mess, it’s human nature to break it. Essentially, what we’re thinking (even less mentally) is, “If the last person doesn’t care about the code, why should I?” “Or” It’s all messy code, who knows who wrote it.”
But that should not be an excuse. Whenever we touch code that used to belong to someone else, we are responsible for that code, and we have consequences if it doesn’t work. To combat this human instinct, we need to take a few small steps to keep our code from getting dirty less often (replacing broken Windows in time).
One simple way is to remove all warnings from the entire package or module we are using. As for unused or annotated code, remove it. If we need this piece of code later, we can always retrieve it from a previous commit in the repository. If there is a warning (such as a primitive type warning) that cannot be resolved directly, annotate the call or method with the @SuppressWarnings annotation. This ensures that we have carefully considered our code: it is not a warning that has been inadvertently issued, but that we have explicitly heeded the warning (such as primitive types).
Once we remove or explicitly disable all warnings, then we must ensure that the code remains warning free. This has two main effects:
- It forces us to think carefully about any code we create.
- Reduce code corruption changes where warnings now lead to errors later.
This has psychological implications for other people, as well as for ourselves — we actually care about the code we’re working on. It’s no longer a one-way street — we force ourselves to change code, commit, and never look back. Instead, we realized that we needed to take responsibility for the code. It also helps later in software development — it shows future developers that this is not a warehouse with broken Windows: it’s a well-maintained code base.
4. Reconstruction
Refactoring has become an important term over the past few decades, and more recently has been used as a shorthand for making any changes to current working code. While refactoring does involve changes to the currently working code, it’s not the whole picture. Martin Fowler, in his important book on this topic, Refactoring, defines Refactoring as:
- Make changes to the internal architecture of the software to make it easier to understand and cheaper to modify without changing its observable behavior.
The key to this definition is that it involves changes that do not change the observable behavior of the system. This means that when we refactor code, we must have a way to ensure that the externally visible behavior of the code does not change. In our case, this means in a test suite that we inherited or developed ourselves. To ensure that we are not changing the external behavior of the system, we must recompile and execute all of our tests every time we make a change.
Also, not every change we make is considered a refactoring. For example, renaming a method to better reflect its intended use is refactoring, but adding new functionality is not. To see the benefits of refactoring, we’ll refactor the SuccessfulFilter. The first refactoring performed was to extract methods to better encapsulate the logic of individual net wages:
Copy the code- public class SuccessfulFilter implements Predicate<Person> {
- @Override
- public boolean test(Person person) {
- return person.getAge() < 30 && getNetSalary(person) > 60000;
- }
- private double getNetSalary(Person person) {
- Return (((person.getsalary () – (250 * 12)) -1500) * 0.94);
- }
- }
After we make this change, we recompile and run our test suite, which continues to pass. It’s easier to see now that success is defined by a Person’s age and net salary, but the getNetSalary method doesn’t seem to be a SuccessfulFilter like the Person class. (The indicator is that the only argument to this method is Person, and the only calls to this method are the Person class methods, Hence the strong affinity for the Person class). To better locate this method, we execute a Move method to Move it to the Person class:
Copy the code- public class Person {
- private int age;
- private double salary;
- public Person(int age, double salary) {
- this.age = age;
- this.salary = salary;
- }
- public void setAge(int age) {
- this.age = age;
- }
- public int getAge() {
- return age;
- }
- public void setSalary(double salary) {
- this.salary = salary;
- }
- public double getSalary() {
- return salary;
- }
- public double getNetSalary() {
- Return ((getSalary() – (250 * 12)) -1500) * 0.94;
- }
- }
- public class SuccessfulFilter implements Predicate<Person> {
- @Override
- public boolean test(Person person) {
- return person.getAge() < 30 && person.getNetSalary() > 60000;
- }
- }
To further clean up this code, we perform the symbolic constant substitution magic Number behavior for each magic number. To find out what these values mean, we may have to talk to the original author or ask someone with enough domain knowledge to steer us in the right direction. We will also perform more extract method refactorings to ensure that the existing methods are as simple as possible.
Copy the code- public class Person {
- private static final int MONTHLY_BONUS = 250;
- private static final int YEARLY_BONUS = MONTHLY_BONUS * 12;
- private static final int YEARLY_BENEFITS_DEDUCTIONS = 1500;
- Private static final double CONTRIBUtion_contribution_percent = 0.06;
- private static final double YEARLY_401K_CONTRIBUTION_MUTLIPLIER = 1 – YEARLY_401K_CONTRIBUTION_PERCENT;
- private int age;
- private double salary;
- public Person(int age, double salary) {
- this.age = age;
- this.salary = salary;
- }
- public void setAge(int age) {
- this.age = age;
- }
- public int getAge() {
- return age;
- }
- public void setSalary(double salary) {
- this.salary = salary;
- }
- public double getSalary() {
- return salary;
- }
- public double getNetSalary() {
- return getPostDeductionSalary();
- }
- private double getPostDeductionSalary() {
- return getPostBenefitsSalary() * YEARLY_401K_CONTRIBUTION_MUTLIPLIER;
- }
- private double getPostBenefitsSalary() {
- return getSalary() – YEARLY_BONUS – YEARLY_BENEFITS_DEDUCTIONS;
- }
- }
- public class SuccessfulFilter implements Predicate<Person> {
- private static final int THRESHOLD_AGE = 30;
- Private static final double THRESHOLD_SALARY = 60000.0;
- @Override
- public boolean test(Person person) {
- return person.getAge() < THRESHOLD_AGE && person.getNetSalary() > THRESHOLD_SALARY;
- }
- }
Recompile and test and find that the system still works as expected: we haven’t changed the external behavior, but we’ve improved the reliability and internal structure of the code. For more complex Refactoring and the Refactoring process, see Martin Fowler’s Refactoring Guru website.
5. The code is better when you leave than when you found it
This last technique is simple in concept but difficult in practice: leave the code better than you found it. When we comb through code, especially someone else’s code, we mostly add functionality, test it, and move on, not caring if we contribute software rot, or if new methods we add to classes cause additional clutter. Therefore, the entire content of this article can be summarized as the following rules:
- Whenever we change the code, make sure you leave it better than you found it.
As mentioned earlier, we are responsible for the damage caused by the class and for the code that changes, and if it doesn’t work, it is our responsibility to fix it. To overcome the entropy that comes with software production, we must force ourselves to leave the code better than we found it. To avoid this problem, we must pay off our technical debt and make sure that the next person who touches the code doesn’t have to pay again. In the future, it may be us who thank ourselves for our persistence at this time.
The author of this article: Xiao Feng original translation
Source: 51 cto
The original link