DEV Community: Fermín Molinuevo

Code Review Best Practices: Improving Collaboration and Code Quality

Fermín Molinuevo — Tue, 05 Mar 2024 20:07:45 +0000

Why code reviews?

Error Detection and Bug Prevention:
Code reviews catch errors and bugs early, preventing them from propagating into the production codebase.
Knowledge Sharing:
Developers gain insights into different areas of the codebase, fostering a cross-functional understanding among team members.
Consistency and Standards:
Code reviews ensure that the codebase adheres to coding standards, promoting consistency and making the codebase more maintainable.
Collaboration and Team Building:
Code reviews promote collaboration, contributing to a shared sense of responsibility for the overall quality of the software.

How to improve code reviews?

Regular and Timely Reviews:
Incorporate code reviews as a standard step in the development workflow using tools like Bitbucket, GitHub, or GitLab Merge Requests to ensure timely and regular assessments.
Clear Objectives and Scope:
Communicate the objectives of the code review using comments and in-app discussions within the version control platform, setting the stage for a focused evaluation.
Automated Tools:
Integrate automated tools like RuboCop for Ruby or ESLint for JavaScript to perform static analysis, linting, and formatting checks before the code reaches the manual review stage.
Evidence:
Provide tangible evidence showcasing the functionality and effectiveness of your code.

Include screenshots of the application or relevant interfaces demonstrating the changes made by the code.
Attach logs that highlight key interactions, errors, or important events during the code’s execution.
Create short screencasts or videos showcasing the behavior of the code in action.
Share results from unit tests to demonstrate the reliability and correctness of the code.
Include performance metrics or benchmarks if relevant to showcase the efficiency of the code changes.
Use descriptive captions or comments to guide reviewers through the evidence and provide context.

This ensures that the evidence is easily understandable and effectively supports the code changes being reviewed.

How to give feedback?

Constructive Criticism:
Frame feedback in a constructive manner using inline comments in version control platforms, focusing on the code’s behavior and maintainability rather than personal attributes.
Specific and Actionable Feedback:
Provide specific comments using tools like Bitbucket’s inline commenting feature, identifying the location of issues and suggesting concrete solutions or improvements for actionable feedback.
Appreciate Positive Aspects:
Acknowledge positive aspects of the code within the review platform, encouraging developers to continue incorporating good practices in their work.

Which tools can I use?

Code Review Tools:
Leverage code review tools like GitHub, GitLab, or Bitbucket to streamline the review process, allowing for comments, discussions, and version control integration.

  - Github: https://github.com/features/code-review
  - GitLab: https://docs.gitlab.com/ee/user/project/merge_requests/
  - Bitbucket: https://support.atlassian.com/bitbucket-cloud/docs/- tutorial-learn-about-bitbucket-pull-requests/
  - Code rabbit (AI): https://coderabbit.ai/
  - Copilot (AI): https://github.com/features/copilot

Checklists:
Use checklists within the review platform to ensure that common issues, including error handling, security considerations, and performance, are consistently addressed during code reviews.

  A - Good Design:

  Evaluate the overall architecture and design choices.
  Check for adherence to design principles and patterns.
  Assess if the code aligns with the project’s architecture and doesn’t introduce unnecessary complexity.

  B — The Functionality Covers User’s Acceptance Criteria:

  Verify that the implemented functionality meets the defined user stories and acceptance criteria.
  Ensure that edge cases and potential scenarios are considered and handled appropriately.

  C — Code Cleanliness and Simplicity:

  Review the code for clarity and simplicity.
  Check if variable and method names are meaningful and self-explanatory.
  Assess if unnecessary complexity and duplication have been minimized.

  D — Avoidance of Unnecessary Future Implementations:

  Examine if the developer has avoided speculative implementations for potential future needs.
  Evaluate if the code addresses the current requirements without over-engineering.

  E — Appropriate Unit Tests:

  Confirm the presence of unit tests for critical and complex parts of the code.
  Assess if tests cover a range of scenarios, including boundary cases and potential error conditions.

  F — Quality of Tests:

  Evaluate the quality of unit tests.
  Ensure that tests are reliable, repeatable, and provide meaningful assertions.
  Check for test coverage, targeting both positive and negative scenarios.

  G — Clear and Useful Comments:

  Assess the clarity and usefulness of comments.
  Ensure that comments focus on explaining the rationale behind decisions, especially where the code might be non-intuitive.
  Verify that comments are up-to-date and not misleading.

  H — Adherence to Project Style Guides:

  Confirm that the code aligns with the project’s coding conventions and style guides.
  Check for consistent indentation, naming conventions, and formatting.
  Ensure that any deviations from the style guide are justified and documented.

Knowledge Sharing Sessions:
Organize knowledge-sharing sessions where team members discuss interesting or challenging code snippets using collaboration tools like Google Meet, Microsoft Teams, or Zoom, fostering a culture of continuous learning and improvement.

How can I monitor them?
Utilize metrics and analytics within version control platforms to monitor the effectiveness of code reviews. Track the number of issues identified, resolution times, and overall code quality improvements.

Several comments or commits may indicate that the code is too complex or the Authors would require more training.
A long review time may indicate that the types of work move slower than other types and can allow you to detect opportunities to accelerate your development cycle.
Few comments and approvers may indicate a lack of available team members or that the code is too complex

Conclusion:
Code reviews, when conducted effectively, contribute significantly to maintaining code quality and fostering collaboration within development teams. By implementing these best practices and leveraging tools like GitHub, GitLab, and Bitbucket, teams can create a positive and constructive code review culture that contributes to the overall success of the software development process. Embrace a continuous improvement mindset, and watch as your codebase becomes more robust and your team more cohesive. Happy coding! 😃

Clean Code Cheatsheet: A Developer’s Guide to Readable and Maintainable Code with Ruby Examples

Fermín Molinuevo — Tue, 05 Mar 2024 20:04:07 +0000

Welcome to this short Clean Code Cheatsheet — your comprehensive guide to writing clean, readable, and maintainable code, with practical examples in the Ruby programming language. Whether you’re a Ruby enthusiast or simply looking to enhance your coding practices, this cheatsheet covers essential clean code principles applicable to any language.

Why Clean Code?

Clean code is the cornerstone of effective software development. It’s about crafting code that is not only functional but also easy to understand, modify, and collaborate on. In the world of programming, code is read far more often than it is written. Clean code prioritizes readability, simplicity, and good design to minimize the long-term cost of maintaining and evolving a codebase.

What to Expect?
This cheatsheet provides actionable tips and guidelines for writing clean code, accompanied by Ruby examples. Each section covers a specific aspect of clean coding, offering insights that can be applied across various programming languages.

Meaningful Names**
Choose descriptive and expressive variable names

# Bad
a = 10
b = 'John'
x = 15

# Good
age = 10
first_name = 'John'
number_of_students = 15

Methods
Craft concise and focused functions

# Bad
def process_data_and_output_result(input)
  # complex data processing
  # ...
  # complex result formatting
  # ...
end

# Good
def process_data(input)
  processed_data = complex_data_processing(input)
  formatted_result = format_result(processed_data)
  output_result(formatted_result)
end

def complex_data_processing(data)
  # detailed logic
end

def format_result(result)
  # detailed formatting
end

def output_result(result)
  # output logic
end

Method arguments
Limit the number of arguments

# Bad - Passing individual parameters
def book(user, hotel)
  # ...
end

user = User.new('John')
hotel = Hotel.new('Hotel')
book(user, hotel)

# Good - Passing a class object
def book(booking_info)
  # ...
end

class BookingInfo
  attr_accessor :user, :hotel

  def initialize(user, hotel)
    @user = user
    @hotel = hotel
  end
end

user = User.new('John')
hotel = Hotel.new('Hotel')

booking_info = BookingInfo.new(user, hotel)
book(booking_info)

Constants
Create constants to avoid the hardcoding things

# Bad
if error_code == 8097
 # Handle the service unavailable error
end

# Good
SERVICE_UNAVAILABLE_ERROR_CODE = 8097

if error_code == SERVICE_UNAVAILABLE_ERROR_CODE
  # Handle the service unavailable error
end

Mental mapping
Avoid the need to keep things on our mind

# Bad
u_n = 'John'
users = ['John', 'Peter']

users.each do |u|
  first_operation(u)
  second_operation
  ..
  n_operation(u)
end

# Good - Using a descriptive loop variable
target_user_name = 'John'
users = ['John', 'Peter']

users.each do |current_user|
  first_operation(current_user)
  second_operation
  # ...
  n_operation(current_user)
end

Comments
Use comments sparingly, focusing on why, not what:

# Bad 
# Increment i by 1 
i += 1  

# Good 
i += 1  # Increment loop counter

Formatting
Follow Code standard formatting conventions.

If you are working with Ruby, try to follow the standard formatting conventions. Refer to the RubyStyle Guide for comprehensive guidelines.

Utilize tools like RuboCop to enforce and maintain the code style

# Bad
def my_method(param1, param2)
param1 + param2
end

# Good
def my_method(param1, param2)
  param1 + param2
end

Error Handling
Use exceptions for error handling.

# Bad
def divide(a, b)
  if b == 0
    puts 'Cannot divide by zero!'
    return
  end
  a / b
end

# Good
class DivisionByZeroError < StandardError
  def initialize(msg = 'Cannot divide by zero!')
    super
  end
end

def divide(a, b)
  raise DivisionByZeroError if b.zero?
  a / b
end

Duplicated Code
Eliminate redundancy

# Bad
def calculate_area_of_circle(radius)
  Math::PI * radius * radius
end

def calculate_volume_of_sphere(radius)
  (4 / 3) * Math::PI * radius * radius * radius
end

# Good
def calculate_area_of_circle(radius)
  Math::PI * radius * radius
end

def calculate_volume_of_sphere(radius)
  (4 / 3.0) * calculate_area_of_circle(radius) * radius
end

Encapsulate conditionals
Always aim for clear and self-explanatory method names to improve the understanding of the code.

# Bad
if user_signed_in? && admin_user?
  # ...
end

# Good
def admin_access?(user)
  user_signed_in? && admin_user?(user)
end

if admin_access?(current_user)
  # ...
end

Avoid negative conditionals
Writing code with positive conditionals instead of negative ones can greatly enhance code readability and maintainability. Code that is expressed in a positive manner tends to be more straightforward and easier for others (or even yourself) to understand.

# Bad
unless !user.active && !user.admin?
  # Perform actions for inactive non-admin users
end

# Good
if user.inactive? && !user.admin?
  # Perform actions for inactive non-admin users
end

Using Polymorphism to Improve Switch Statements
Switch statements can become hard to maintain and extend as the number of cases grows. Leveraging polymorphism provides a more flexible and scalable alternative. Let’s explore how to improve a switch statement using polymorphism with an example.

# Bad

class Shape
  def draw
    case type
    when :circle
      draw_circle
    when :square
      draw_square
    when :triangle
      draw_triangle
    else
      raise 'Unsupported shape!'
    end
  end

private

  def draw_circle
    # draw circle logic
  end

  def draw_square
    # draw square logic
  end

  def draw_triangle
    # draw triangle logic
  end
end

shape = Shape.new(:circle)
shape.draw


# Good

class Shape
  def draw
    raise NotImplementedError, 'Subclasses must implement this method'
  end
end

class Circle < Shape
  def draw
    # draw circle logic
  end
end

class Square < Shape
  def draw
    # draw square logic
  end
end

class Triangle < Shape
  def draw
    # draw triangle logic
  end
end

shape = Circle.new
shape.draw

Objects and Data Structures
Encapsulate data with behavior in classes, and use data structures for simple storage

# Bad
user_data = { name: 'Alice', age: 25 }

# Good
class User
  attr_accessor :name, :age
  def initialize(name, age)
    @name = name
    @age = age
  end
end

SOLID Principles
Follow SOLID principles for designing maintainable and scalable systems

Single Responsibility Principle (SRP)
The Single Responsibility Principle states that a class should have only one reason to change. In this example, the Calculator class has a single responsibility, which is to perform addition. If there are changes related to addition, the Calculator class is the only one that needs modification.

class Calculator
  def add(a, b)
    a + b
  end
end

Open/Closed Principle (OCP)
It states that a class should be open for extension but closed for modification.

class Shape
  def area
    raise NotImplementedError, 'Subclasses must implement this method'
  end
end

class Circle < Shape
  def area(radius)
    Math::PI * radius * radius
  end
end

The Shape class is open for extension by allowing new shapes to be added through subclasses (like Circle) without modifying the existing Shape class.

Liskov Substitution Principle (LSP)
The Liskov Substitution Principle states that objects of a superclass should be able to be replaced with objects of a subclass without affecting the correctness of the program.

class Shape
  def area
    raise NotImplementedError, 'Subclasses must implement this method'
  end
end

class Rectangle < Shape
  attr_accessor :width, :height

  def area
    width * height
  end
end

class Square < Shape
  attr_accessor :side

  def area
    side * side
  end
end

def calculate_total_area(shapes)
  total_area = 0
  shapes.each { |shape| total_area += shape.area }
  total_area
end

rectangle = Rectangle.new
rectangle.width = 5
rectangle.height = 10

square = Square.new
square.side = 7

shapes = [rectangle, square]

puts "Total area: #{calculate_total_area(shapes)}"

Interface Segregation Principle (ISP)
The Interface Segregation Principle states that a class should not be forced to implement interfaces it does not use.

# Bad - Interface with multiple methods
module Employee
  def work
    # work logic
  end

  def manage
    # management logic
  end
end

class Worker
  include Employee

  # Worker needs to implement both work and manage, even if it doesn't manage
end

class Manager
  include Employee

  # Manager needs to implement both work and manage, even if it doesn't perform the regular work
end

# Good - Separate interfaces for work and management
module Workable
  def work
    # work logic
  end
end

module Managable
  def manage
    # management logic
  end
end

class Worker
  include Workable

  # Worker only includes the Workable interface, as it doesn't manage
end

class Manager
  include Workable
  include Managable

  # Manager includes both interfaces, as it performs both work and management
end

Here, each class has a specific interface relevant to its responsibilities. Workers implement the work interface, while Managers implement the manage interface.

Dependency Inversion Principle (DIP)
The Dependency Inversion Principle states that high-level modules should not depend on low-level modules, but both should depend on abstractions.

class LightBulb
  def turn_on
    # turn on logic
  end
end

class Switch
  def initialize(device)
    @device = device
  end
  def operate
    @device.turn_on
  end
end

The Switch class depends on the abstraction (LightBulb) rather than the specific implementation. This allows for flexibility and easier maintenance.

Testing

Descriptive Test Names: To help developers understand the purpose and expected behavior of each test case without diving into the details.

# Bad
def test_method
  # ...
end

# Good
def test_user_authentication_success
  # ...
end

**Explicit assertions**
Use explicit assertions that convey the expected outcome of the test, reducing ambiguity and making it clear what the test is checking.

# Bad
assert_equal true, user.authenticate('password')

# Good
assert user.authenticate('password'), 'User authentication failed'

This cheatsheet provides a foundation for writing clean code in any programming language, using Ruby as a practical example. Dive into each section for more detailed guidance and examples. Let’s make our code not only functional but a joy to read and maintain.

Happy coding!

Clean Code Cheatsheet: A Developer’s Guide to Readable and Maintainable Code with Ruby Examples

Fermín Molinuevo — Tue, 05 Mar 2024 19:55:33 +0000

Why Clean Code?
Clean code is the cornerstone of effective software development. It’s about crafting code that is not only functional but also easy to understand, modify, and collaborate on. In the world of programming, code is read far more often than it is written. Clean code prioritizes readability, simplicity, and good design to minimize the long-term cost of maintaining and evolving a codebase.

What to Expect
This cheatsheet provides actionable tips and guidelines for writing clean code, accompanied by Ruby examples. Each section covers a specific aspect of clean coding, offering insights that can be applied across various programming languages.

Meaningful Names
Choose descriptive and expressive variable names

`# Bad
a = 10
b = 'John'
x = 15

Good

age = 10
first_name = 'John'
number_of_students = 15`

Methods
Craft concise and focused functions
`

Bad

def process_data_and_output_result(input)
# complex data processing
# ...
# complex result formatting
# ...
end

Good

def process_data(input)
processed_data = complex_data_processing(input)
formatted_result = format_result(processed_data)
output_result(formatted_result)
end

def complex_data_processing(data)
# detailed logic
end

def format_result(result)
# detailed formatting
end

def output_result(result)
# output logic
end`

Method arguments
Limit the number of arguments

`# Bad - Passing individual parameters
def book(user, hotel)
# ...
end

user = User.new('John')
hotel = Hotel.new('Hotel')
book(user, hotel)

Good - Passing a class object

def book(booking_info)
# ...
end

class BookingInfo
attr_accessor :user, :hotel

def initialize(user, hotel)
@user = user
@hotel = hotel
end
end

user = User.new('John')
hotel = Hotel.new('Hotel')

booking_info = BookingInfo.new(user, hotel)
book(booking_info)`

Constants
Create constants to avoid the hardcoding things

`# Bad
if error_code == 8097
# Handle the service unavailable error
end

Good

SERVICE_UNAVAILABLE_ERROR_CODE = 8097

if error_code == SERVICE_UNAVAILABLE_ERROR_CODE
# Handle the service unavailable error
end`

Mental mapping
Avoid the need to keep things on our mind

`# Bad
u_n = 'John'
users = ['John', 'Peter']

users.each do |u|
first_operation(u)
second_operation
..
n_operation(u)
end

Good - Using a descriptive loop variable

target_user_name = 'John'
users = ['John', 'Peter']

users.each do |current_user|
first_operation(current_user)
second_operation
# ...
n_operation(current_user)
end`

Comments
Use comments sparingly, focusing on why, not what:

`# Bad

Increment i by 1

i += 1

Good

i += 1 # Increment loop counter`

Formatting
Follow Code standard formatting conventions.

If you are working with Ruby, try to follow the standard formatting conventions. Refer to the RubyStyle Guide for comprehensive guidelines.

Utilize tools like RuboCop to enforce and maintain the code style

`# Bad
def my_method(param1, param2)
result = param1 + param2
return result
end

Good

def my_method(param1, param2)
result = param1 + param2
return result
end`

Error Handling
Use exceptions for error handling.

`# Bad
def divide(a, b)
if b == 0
puts 'Cannot divide by zero!'
return
end
a / b
end

Good

class DivisionByZeroError < StandardError
def initialize(msg = 'Cannot divide by zero!')
super
end
end

def divide(a, b)
raise DivisionByZeroError if b.zero?
a / b
end`

Duplicated Code
Eliminate redundancy

`# Bad
def calculate_area_of_circle(radius)
Math::PI * radius * radius
end

def calculate_volume_of_sphere(radius)
(4 / 3) * Math::PI * radius * radius * radius
end

Good

def calculate_area_of_circle(radius)
Math::PI * radius * radius
end

def calculate_volume_of_sphere(radius)
(4 / 3.0) * calculate_area_of_circle(radius) * radius
end`

Encapsulate conditionals
Always aim for self-explanatory method names to improve the understanding of the code.

`# Bad
if user_signed_in? && admin_user?
# ...
end

Good

def admin_access?(user)
user_signed_in? && admin_user?(user)
end

if admin_access?(current_user)
# ...
end`

Avoid negative conditionals
Writing code with positive conditionals instead of negative ones can greatly enhance code readability and maintainability. Code that is expressed in a positive manner tends to be more straightforward and easier for others (or even yourself) to understand.

`# Bad
unless !user.active && !user.admin?
# Perform actions for inactive non-admin users
end

Good

if user.inactive? && !user.admin?
# Perform actions for inactive non-admin users
end`

Using Polymorphism to Improve Switch Statements
Switch statements can become hard to maintain and extend as the number of cases grows. Leveraging polymorphism provides a more flexible and scalable alternative. Let’s explore how to improve a switch statement using polymorphism with an example.

`# Bad

class Shape
def draw
case type
when :circle
draw_circle
when :square
draw_square
when :triangle
draw_triangle
else
raise 'Unsupported shape!'
end
end

private

def draw_circle
# draw circle logic
end

def draw_square
# draw square logic
end

def draw_triangle
# draw triangle logic
end
end

shape = Shape.new(:circle)
shape.draw

Good

class Shape
def draw
raise NotImplementedError, 'Subclasses must implement this method'
end
end

class Circle < Shape
def draw
# draw circle logic
end
end

class Square < Shape
def draw
# draw square logic
end
end

class Triangle < Shape
def draw
# draw triangle logic
end
end

shape = Circle.new
shape.draw
Objects and Data Structures
Encapsulate data with behavior in classes, and use data structures for simple storage

Bad

user_data = { name: 'Alice', age: 25 }

Good

class User
attr_accessor :name, :age
def initialize(name, age)
@name = name
@age = age
end
end`

SOLID Principles
Follow SOLID principles for designing maintainable and scalable systems

Single Responsibility Principle (SRP): The Single Responsibility Principle states that a class should have only one reason to change. In this example, the Calculator class has a single responsibility, which is to perform addition. If there are changes related to addition, the Calculator class is the only one that needs modification.

class Calculator def add(a, b) a + b end end

Open/Closed Principle (OCP) : It states that a class should be open for extension but closed for modification.

`class Shape
def area
raise NotImplementedError, 'Subclasses must implement this method'
end
end

class Circle < Shape
def area(radius)
Math::PI * radius * radius
end
end`

The Shape class is open for extension by allowing new shapes to be added through subclasses (like Circle) without modifying the existing Shape class.

Liskov Substitution Principle (LSP): The Liskov Substitution Principle states that objects of a superclass should be able to be replaced with objects of a subclass without affecting the correctness of the program.

`class Shape
def area
raise NotImplementedError, 'Subclasses must implement this method'
end
end

class Rectangle < Shape
attr_accessor :width, :height

def area
width * height
end
end

class Square < Shape
attr_accessor :side

def area
side * side
end
end

def calculate_total_area(shapes)
total_area = 0
shapes.each { |shape| total_area += shape.area }
total_area
end

rectangle = Rectangle.new
rectangle.width = 5
rectangle.height = 10

square = Square.new
square.side = 7

shapes = [rectangle, square]

puts "Total area: #{calculate_total_area(shapes)}"`

Interface Segregation Principle (ISP): The Interface Segregation Principle states that a class should not be forced to implement interfaces it does not use.

`# Bad - Interface with multiple methods
module Employee
def work
# work logic
end

def manage
# management logic
end
end

class Worker
include Employee

# Worker needs to implement both work and manage, even if it doesn't manage
end

class Manager
include Employee

# Manager needs to implement both work and manage, even if it doesn't perform the regular work
end

Good - Separate interfaces for work and management

module Workable
def work
# work logic
end
end

module Managable
def manage
# management logic
end
end

class Worker
include Workable

# Worker only includes the Workable interface, as it doesn't manage
end

class Manager
include Workable
include Managable

# Manager includes both interfaces, as it performs both work and management
end`

Here, each class has a specific interface relevant to its responsibilities. Workers implement the work interface, while Managers implement the manage interface.

Dependency Inversion Principle (DIP): The Dependency Inversion Principle states that high-level modules should not depend on low-level modules, but both should depend on abstractions.
`
class LightBulb
def turn_on
# turn on logic
end
end

class Switch
def initialize(device)
@device = device
end
def operate
@device.turn_on
end
end`

he Switch class depends on the abstraction (LightBulb) rather than the specific implementation. This allows for flexibility and easier maintenance.

Testing
Descriptive Test Names: To help developers understand the purpose and expected behavior of each test case without diving into the details.
`

Bad

def test_method
# ...
end

Good

def test_user_authentication_success
# ...
end
Explicit assertions: Use explicit assertions that convey the expected outcome of the test, reducing ambiguity and making it clear what the test is checking.

Bad

assert_equal true, user.authenticate('password')

Good

assert user.authenticate('password'), 'User authentication failed'`

Happy coding!