Pull Request reviews and checkpoints to success
Being part of the digital agency as an Developer, I've been working in the multi-cultural teams across the world, and found different personas and developers when it comes to the point of the PR review.
It makes a lot of importance to compose yourself, take your time, and review done changes, becomes no one is perfect, everyone makes mistakes, and done changes should be bulletproof for the rest of the features, and enable you to reuse them as one of the most done modular features.
Through this blog I will let you through 21 checkpoints, which all of you may or may not consider importmant, it's all about team setup, team consolidation, and finally conversation on the pull request, to go through, or debate about possible changes. In the end, both reviewer and PR creator, are working for the same team, same goal.
1. Duplicated code
Summary
One of the most common smells in code because it's too easy to copy code.
Code duplication can be a silent enemy that creeps into our projects, leading to maintainability issues, increased bug potential, and wasted developer effort. The "Pull Request Review: Duplicated Code" is a critical phase in the software development lifecycle where the team identifies and resolves code duplication issues.
Purpose
The primary purpose of this pull request review is to eliminate duplicated code segments, ensuring that our codebase remains clean, efficient, and maintainable. By consolidating identical or similar code fragments, we reduce the risk of inconsistencies and make it easier to implement future changes or updates.
Benefits
Codebase health
Eliminating duplicated code improves the overall health of the codebase. It reduces the risk of inconsistent behavior and simplifies future maintenance.
Efficiency
Code duplication is often a source of inefficiency. By centralizing functionality, we optimize resource usage and reduce execution time.
Readability
Consolidated code is easier to read and understand, making it simpler for developers to work with and maintain.
Bug Reduction
Duplicated code is a breeding ground for bugs. Removing redundancy reduces the likelihood of introducing new issues.
2. Long methods
Summary
Methods longer than 20-30 lines (actually with more than 1 responsibility).
Long methods can lead to a range of issues, including reduced code readability, increased complexity, and difficulties in debugging and testing. This review process is designed to identify, refactor, and optimize long methods to improve overall code health.
Purpose
The primary purpose of this pull request review is to break down long, complex methods into smaller, more manageable pieces. This refactoring not only enhances code readability but also promotes code reuse and modularity, making it easier to maintain, test, and debug.
Benefits
Improved code readability
Smaller methods are easier to read and understand, making the codebase more accessible to developers.
Enhanced maintainability
Breaking down long methods simplifies maintenance tasks and reduces the risk of introducing bugs during modifications.
Modularity
Smaller methods can be reused in various parts of the codebase, promoting modularity and reducing redundancy.
Easier Debugging
Smaller methods make it simpler to pinpoint issues during debugging, as the scope of each method is limited.
3. Large classes
Summary
Large classes have many responsibilities and over a thousand lines of code.
Large classes can be a source of numerous challenges, including decreased code maintainability, reduced readability, and hindered collaboration among developers. This review process aims to dissect and optimize large classes to enhance code quality and long-term project sustainability.
Purpose
The primary purpose of this pull request review is to break down large classes into smaller, more manageable units. By refactoring large classes, we can achieve improved code organization, readability, and modularity, making the codebase easier to maintain, extend, and troubleshoot.
Benefits
Enhanced Code Organization: Smaller classes contribute to improved code organization and reduce cognitive load, making it easier for developers to navigate and understand the codebase.
Better Maintainability: Breaking down large classes simplifies maintenance tasks and mitigates the risk of introducing errors during modifications.
Promotes Reusability: Smaller, focused classes can be reused across different parts of the codebase, promoting code modularity and reducing redundancy.
Facilitates Collaboration: Smaller classes make it easier for multiple developers to work on different components of the codebase concurrently.
4. Long parameter list
Summary
This can be a sign that the method does too much.
Long parameter lists can lead to reduced code readability, increased complexity, and a higher likelihood of introducing bugs during development. This review process aims to refactor long parameter lists, improving code quality and making our software more maintainable.
Purpose
The primary purpose of this pull request review is to streamline and simplify methods or functions by reducing the number of parameters they accept. This refactoring enhances code readability, maintainability, and robustness, making it easier to work with and modify in the future.
Benefits
Improved Code Readability
Simplified parameter lists make code more readable and self-explanatory, reducing the need for extensive comments.
Enhanced Maintainability
Refactoring long parameter lists simplifies maintenance tasks and reduces the risk of introducing errors during changes or additions.
Encourages Best Practices
Promotes adherence to best practices such as the Single Responsibility Principle, improving code structure and quality.
Facilitates Code Reuse
Reducing parameter lists can promote the reuse of methods or functions in different contexts, fostering code modularity.
5. Divergent change
Summary
Code smell that happens when you change the same code for different reasons.
Divergent change is often a sign of code that lacks cohesion and is difficult to maintain. This review process is designed to refactor such divergent change, improving code quality, and fostering better code organization.
Purpose
The primary purpose of this pull request review is to restructure code to ensure that each module or class adheres to the Single Responsibility Principle. By isolating and addressing divergent changes, we enhance code cohesion, readability, and maintainability, ensuring our software remains adaptable to evolving requirements.
Benefits
Enhanced Code Cohesion
Refactoring divergent change results in code that is more cohesive, with each module or class focused on a single responsibility.
Improved Readability
Code that adheres to the Single Responsibility Principle is easier to read, understand, and maintain.
Easier Debugging
Clearer separation of responsibilities makes it simpler to pinpoint and resolve issues.
Promotes Scalability
Code structured around single responsibilities is more adaptable to changes and additions.
6. Shotgun surgery
Summary
Occurs when you need to make changes in different parts of the codebase to implement a feature or fix a bug.
Purpose
"Shotgun Surgery" phase is a critical step in our software development process aimed at identifying and mitigating instances of shotgun surgery in our codebase. Shotgun surgery refers to a situation where a single code change necessitates multiple modifications across the codebase, leading to increased complexity, higher maintenance costs, and a higher risk of introducing errors. This review process is designed to refactor code to reduce the impact of shotgun surgery and promote better code organization.
The primary purpose of this pull request review is to refactor code to minimize the ripple effect of changes. By isolating and addressing shotgun surgery scenarios, we enhance code modularity, maintainability, and reduce the risk of unintended side effects when making updates.
Benefits
Reduced Code Coupling
Refactoring to address shotgun surgery reduces code coupling and promotes modularity, making it easier to maintain and extend the codebase.
Lower Maintenance Costs
Code that is less prone to shotgun surgery requires less effort and time for maintenance, resulting in cost savings.
Easier Debugging
Isolating responsibilities makes it simpler to identify the source of issues and reduces the risk of unintended side effects.
Improved Code Quality
Promoting code organization and reducing dependencies leads to improved overall code quality.
7. Feature envy
Summary
Code that accesses data or methods from another class more often than its own.
Feature envy occurs when one class or module is excessively interested in the internal details or data of another, potentially leading to decreased code cohesion, increased complexity, and maintainability challenges. This review process is designed to refactor code and mitigate feature envy, improving code quality and fostering better code organization.
Purpose
The primary purpose of this pull request review is to restructure code to ensure that responsibilities are appropriately distributed among classes or modules. By isolating and addressing feature envy, we enhance code cohesion, readability, and maintainability, ensuring our software remains adaptable and easier to work with.
Benefits
Enhanced Code Cohesion
Refactoring feature envy leads to code with higher cohesion, where each class or module has well-defined responsibilities.
Improved Code Readability:
Code that adheres to proper encapsulation is easier to read, understand, and maintain.
Easier Debugging
Clear separation of responsibilities makes it simpler to pinpoint and resolve issues.
Promotes Scalability:
Code structured around appropriate responsibilities is more adaptable to changes and additions.
8. Data clumps
Summary
Occurs when three or more data items always appear together in the code
Data clumps refer to repeated groups of closely related data fields or parameters that often appear together, indicating a potential design issue. This review process is designed to refactor code and address data clumps, improving code quality, maintainability, and code organization.
Purpose
The primary purpose of this pull request review is to refactor code to remove data clumps by introducing more structured data representations. By isolating and addressing data clumps, we enhance code readability, reduce redundancy, and make our software more adaptable to evolving requirements.
Benefits
Enhanced Code Organization
Refactoring data clumps leads to a more organized codebase with structured data representations, improving code maintainability and readability.
Reduced Redundancy
By encapsulating related data, we reduce redundancy and the likelihood of inconsistent data updates.
Simplified Maintenance
Code that addresses data clumps is easier to maintain and extend, as changes to data structures are centralized.
Promotes Scalability
Structured data types are more adaptable to changes and facilitate future code enhancements.
9. Primitive obsession
Summary
Not substituting particular primitive types with classes.
Primitive obsession occurs when simple data types, such as integers or strings, are used excessively in place of more structured and meaningful objects. This review process aims to refactor code and address primitive obsession, enhancing code quality, maintainability, and code organization.
Purpose
The primary purpose of this pull request review is to refactor code to replace primitive data types with more meaningful and structured objects. By isolating and addressing primitive obsession, we improve code readability, reduce redundancy, and make our software more adaptable to evolving requirements.
Benefits
Improved Code Clarity
Refactoring primitive obsession results in code that is more self-explanatory, as data is represented in a more meaningful and structured way.
Reduced Redundancy
By introducing structured objects, we reduce redundancy and the likelihood of inconsistent data updates.
Simplified Maintenance
Code that addresses primitive obsession is easier to maintain and extend, as data structures are more cohesive and encapsulated.
Promotes Code Reusability
Structured objects encourage code reusability and are more adaptable to changes.
10. Switch statements
Summary
Having the same switch statement in multiple places in the code
Switch statements can lead to code that is hard to maintain, less extensible, and more prone to errors when not used judiciously. This review process aims to refactor code and address switch statements, improving code quality, maintainability, and code organization.
Purpose
The primary purpose of this pull request review is to refactor code to reduce the reliance on switch statements, replacing them with more maintainable, extensible, and robust alternatives. By isolating and addressing switch statements, we enhance code readability, scalability, and adaptability to evolving requirements.
Benefits
Enhanced Code Maintainability
Refactoring switch statements results in code that is easier to maintain and extend, as logic is encapsulated in more meaningful abstractions.
Improved Code Readability
Code that replaces switch statements is more self-explanatory and easier to understand.
Reduced Error-Prone Code
The use of more structured abstractions reduces the risk of introducing errors when making changes or additions.
Promotes Code Reusability
Replacing switch statements often leads to code that is more adaptable and promotes code reusability.
11. Parallel inheritance hierarchies
Summary
Every time you make one subclass or implement one interface, you also need to make another subclass or create a class that implements another interface
Parallel inheritance hierarchies occur when two or more class hierarchies evolve independently, potentially leading to code duplication, reduced code maintainability, and increased complexity. This review process is designed to refactor code and reconcile parallel inheritance hierarchies, improving code quality, maintainability, and code organization.
Purpose
The primary purpose of this pull request review is to refactor code to eliminate parallel inheritance hierarchies by promoting code reuse and reducing redundancy. By isolating and addressing parallel inheritance hierarchies, we enhance code readability, maintainability, and adaptability to evolving requirements.
Benefits
Enhanced Code Maintainability
Refactoring parallel inheritance hierarchies results in code that is easier to maintain and extend, as code is more organized and reduces redundancy.
Improved Code Readability
Code that eliminates parallel inheritance hierarchies is more self-explanatory and easier to understand.
Reduced Code Duplication
The refactoring process reduces code duplication, leading to a more efficient and error-resistant codebase.
Promotes Code Reusability
Eliminating parallel inheritance hierarchies promotes code reusability and ensures that changes are made in a centralized manner.
12. Lazy class
Summary
A lazy class is simply a class with one or two methods that you could move to another class
Lazy classes can lead to code bloat, reduced maintainability, and increased complexity. This review process aims to refactor or remove these underutilized components, improving code quality, readability, and maintainability.
Purpose
The primary purpose of this pull request review is to evaluate the relevance and necessity of existing classes or components in the codebase. By identifying and addressing lazy classes, we aim to enhance code cleanliness, minimize complexity, and streamline our software.
Benefits
Leaner Codebase:
Removing or refactoring lazy classes reduces code bloat and complexity, making the codebase more efficient and maintainable.
Improved Readability
Code that eliminates irrelevant classes is more straightforward and easier to understand.
Simplified Maintenance
A smaller, focused codebase is easier to maintain and extend, reducing the risk of introducing errors.
Promotes Code Clarity
Removing or optimizing lazy classes promotes code clarity and helps developers focus on essential functionality.
13. Speculative generality
Summary
Code that is over engineered.
Speculative generality occurs when code includes unnecessary abstractions, design patterns, or features that are not currently required and might never be. This review process aims to refactor code and eliminate such unnecessary complexities, thereby improving code quality, maintainability, and clarity.
Purpose
The primary purpose of this pull request review is to evaluate the relevance and necessity of existing abstractions, patterns, or features in the codebase. By identifying and addressing speculative generality, we aim to simplify the code, reduce complexity, and make our software more efficient and maintainable.
Benefits
Simpler Codebase
Removing speculative generality simplifies the code, making it more efficient and easier to maintain.
Improved Readability
Code that eliminates unnecessary complexities is more straightforward and easier to understand.
Simplified Maintenance
A leaner codebase is easier to maintain and extend, reducing the risk of introducing errors.
Efficiency Gains
Reducing unnecessary abstractions and features can lead to performance improvements.
14. Temporary field
Summary
Adds complexity by unnecessarily storing a result into a variable, instead of returning the result itself
Temporary fields are variables or properties that are used for a short duration, often within a limited scope, and may not serve a long-term purpose. This review process aims to refactor code to eliminate or better manage temporary fields, thereby improving code quality, maintainability, and code organization.
Purpose
The primary purpose of this pull request review is to evaluate the relevance and necessity of temporary fields in the codebase. By identifying and addressing temporary fields, we aim to enhance code clarity, reduce redundancy, and make our software more efficient and maintainable.
Benefits
Simplified Codebase
Removing or better managing temporary fields reduces code complexity and redundancy, making it more efficient and maintainable.
Improved Code Readability
Code that eliminates unnecessary fields is more straightforward and easier to understand.
Reduced Risk of Errors
Eliminating temporary fields reduces the risk of introducing errors when making changes or additions.
Promotes Code Clarity
Properly managing data within the codebase improves code clarity and simplifies troubleshooting.
15. Message chains
Summary
This is a class that calls an object which calls another object: customer.LastInvoice.Order.OrderId
Message chains occur when one object sends a chain of method calls to another object, often resulting in tight coupling and reduced code maintainability. This review process aims to refactor code to eliminate or simplify message chains, thereby improving code quality, readability, and maintainability.
Purpose
The primary purpose of this pull request review is to evaluate the complexity and necessity of message chains in the codebase. By identifying and addressing message chains, we aim to enhance code cohesion, reduce coupling, and make our software more adaptable to changes.
Benefits
Enhanced Code Cohesion
Refactoring or simplifying message chains results in code with higher cohesion, where behavior is encapsulated more appropriately.
Improved Code Readability
Code that eliminates complex message chains is more straightforward and easier to understand.
Reduced Code Coupling
Properly managing message chains reduces tight coupling between objects, making the codebase more adaptable to changes.
Promotes Code Maintainability
Code that avoids excessive message chains is easier to maintain and extend, reducing the risk of introducing errors.
16. Middle man
Summary
A class that only delegates calls to other classes
Middle man code occurs when an intermediary class or component simply delegates calls to another, providing little or no additional functionality. This review process aims to refactor code to eliminate or streamline middle man components, thereby improving code quality, readability, and maintainability.
Purpose
The primary purpose of this pull request review is to evaluate the necessity and efficiency of middle man components in the codebase. By identifying and addressing middle men, we aim to enhance code simplicity, reduce redundancy, and make our software more efficient.
Benefits
Simplified Codebase
Removing or optimizing middle man components simplifies the codebase, reducing redundancy and making it more efficient.
Improved Code Readability
Code that eliminates unnecessary intermediaries is more straightforward and easier to understand.
Reduced Code Redundancy
Properly managing middle man code reduces redundancy, leading to a more efficient and error-resistant codebase.
Promotes Code Clarity
Optimizing middle men promotes code clarity and helps developers focus on essential functionality.
17. Inappropriate intimacy
Summary
This can happen when classes are tightly coupled, meaning they rely on each other too much
Inappropriate intimacy refers to excessive coupling or dependencies between classes or components that share too much information, making the code less maintainable, less modular, and more error-prone. This review process aims to refactor code to eliminate or reduce inappropriate intimacy, thereby improving code quality, maintainability, and code organization.
Purpose
The primary purpose of this pull request review is to evaluate the level of coupling and dependencies between classes or components in the codebase. By identifying and addressing inappropriate intimacy, we aim to enhance code modularity, reduce coupling, and make our software more adaptable to changes.
Benefits
Enhanced Code Modularity
Refactoring inappropriate intimacy leads to code that is more modular and encapsulated, making it easier to maintain and extend.
Reduced Code Coupling
Properly managing dependencies between components reduces tight coupling, making the codebase more adaptable to changes.
Improved Code Readability
Code that eliminates inappropriate intimacy is more straightforward and easier to understand.
Promotes Code Reusability
Decoupling components often leads to code that is more reusable and adaptable.
18. Alternative classes with different interfaces
Summary
Classes that have different method signatures but almost the same behavior
Alternative Classes with Different Interfaces phase is an essential step in our software development process dedicated to identifying and resolving instances where alternative classes exhibit different interfaces or methods, potentially leading to confusion, maintenance challenges, and code inconsistencies. This review process aims to refactor code to align the interfaces of alternative classes or provide clearer naming and documentation to ensure better code quality, consistency, and maintainability.
Purpose
The primary purpose of this pull request review is to evaluate the interfaces and naming of alternative classes within the codebase. By identifying and addressing differences in interfaces, we aim to improve code clarity, reduce confusion, and enhance maintainability.
Benefits
Enhanced Code Consistency
Aligning interfaces and naming conventions reduces confusion and promotes code consistency, making the codebase more maintainable.
Improved Code Readability
Code that addresses differences in interfaces is more straightforward and easier to understand.
Reduced Risk of Errors
Consistent interfaces reduce the risk of introducing errors when working with alternative classes.
Promotes Code Reusability
Aligning interfaces often leads to code that is more reusable and adaptable.
19. Refused request
Summary
Happens when subclasses don't use methods and fields inherited from the parent class
Refused Request phase is a crucial step in our software development process aimed at identifying and resolving instances where a class or component refuses to fulfill certain requests from clients, causing inefficiencies and potential issues in the codebase. This review process focuses on refactoring code to ensure that the class or component appropriately handles all expected requests, leading to better code quality, robustness, and client satisfaction.
Purpose
The primary purpose of this pull request review is to evaluate the behavior of classes or components in handling client requests. By identifying and addressing refused requests, we aim to improve code robustness, client interactions, and overall software reliability.
Benefits
Enhanced Code Robustness
Refactoring to handle refused requests improves code robustness, ensuring that the class or component can effectively serve clients' needs.
Improved Client Interactions
Addressing refused requests leads to better client interactions, reducing frustration and misunderstandings.
Reduced Risk of Errors
Improved error handling and responses reduce the risk of unexpected issues when clients interact with the software.
Enhanced Software Reliability
Robust handling of client requests contributes to the overall reliability and performance of the software.
20. Comments
Summary
Comments play a vital role in enhancing code understanding, maintainability, and collaboration. This review process aims to ensure that code comments are clear, concise, and provide valuable insights to developers, ultimately improving code quality, documentation, but are not overused.
Purpose
The primary purpose of this pull request review is to assess the quality and appropriateness of comments within the codebase. By evaluating and enhancing code comments, we aim to enhance code readability, maintainability, and the overall development experience.
Benefits
Enhanced Code Readability
Clear, concise, and relevant comments improve code understanding, making it easier for developers to work with the codebase.
Improved Code Maintenance
Well-documented code reduces the time and effort required for maintenance and troubleshooting.
Effective Collaboration
High-quality comments facilitate collaboration among team members, fostering knowledge sharing and best practices.
Legal Compliance
Accurate license and copyright notices within comments help ensure legal compliance.
21. Dead code
Summary
Code that is no longer used.
Dead Code Elimination" phase is a crucial step in our software development process dedicated to identifying and removing dead or unused code within our codebase. Dead code, which serves no purpose and adds unnecessary complexity, can lead to maintenance challenges and increase the risk of introducing errors. This review process aims to refactor or remove such code, improving code quality, performance, and maintainability.
Purpose
The primary purpose of this pull request review is to evaluate the codebase for the presence of dead or unused code and eliminate it. By identifying and addressing dead code, we aim to simplify the codebase, reduce complexity, and enhance overall software health.
Benefits
Simplified Codebase
Removing dead code reduces code complexity, making it easier to understand and maintain.
Improved Code Readability
A codebase free of unused code is more straightforward and easier to work with.
Reduced Risk of Errors
Eliminating dead code reduces the risk of introducing errors when making changes or additions.
Performance Gains
Dead code removal can lead to performance improvements, as unnecessary computations are eliminated.
