In the journey of code refinement, where intricate designs meet human ingenuity, few processes are as enlightening as the code review. Over time, I’ve harnessed wisdom from my experiences to fine-tune this essential practice.
In this exploration, we shall analyze code together, identifying key aspects of professional and respectful code review.
Let’s embark on this journey of code review wisdom.
Identifying Nuanced Errors
Initially, seek out frequent oversights—those small yet impactful errors developers often miss.
public class OrderProcessor
{
private readonly List<Order> _orders;
public OrderProcessor(List<Order> orders)
{
_orders = orders;
}
public void ProcessOrders()
{
foreach (var order in _orders)
{
if (order.Status == "Paid")
{
order.Status = "Shipped";
Console.WriteLine($"Order {order.Id} is now Shipped.");
}
else if (order.orderStatus== "Shipped")
{
order.Status = "Completed";
Console.WriteLine($"Order {order.Id} is now Completed.");
}
}
}
}
public class Order
{
public int order_id { get; set; }
public string orderStatus { get; set; }
public decimal TotalAmount{ get; set; }
}
Reflect on potential issues in the code above. Did you notice?
- Absence of exception handling could disrupt stability.
- The hardcoding of statuses makes the code brittle.
- Inconsistent naming conventions lead to muddled code.
Observe the refined version where we integrate enums and maintain uniformity:
using System;
using System.Collections.Generic;
public class OrderProcessor
{
private readonly List<Order> _orders;
public OrderProcessor(List<Order> orders)
{
_orders = orders;
}
public void ProcessOrders()
{
foreach (var order in _orders)
{
try
{
if (order.Status == OrderStatus.Paid)
{
order.Status = OrderStatus.Shipped;
Console.WriteLine($"Order {order.Id} is now Shipped.");
}
else if (order.Status == OrderStatus.Shipped)
{
order.Status = OrderStatus.Completed;
Console.WriteLine($"Order {order.Id} is now Completed.");
}
else
{
Console.WriteLine($"Order {order.Id} has an unknown status: {order.Status}");
}
}
catch (Exception ex)
{
Console.WriteLine($"Error processing order {order.Id}: {ex.Message}");
}
}
}
}
public class Order
{
public int Id { get; set; }
public OrderStatus Status { get; set; }
public decimal TotalAmount { get; set; }
}
public enum OrderStatus
{
Paid,
Shipped,
Completed
}
Recognizing Subtle Omissions
Analyzing the next example, do not neglect the presence of null checks, a common oversight when expediency trumps thoroughness.
public class OrderProcessor
{
public void GetCustomerOrderDetails(Customer customer)
{
if (customer.Orders.Count > 0)
{
foreach (var order in customer.Orders)
{
Console.WriteLine($"Order ID: {order.OrderId}, Amount: {order.Amount}");
}
}
else
{
Console.WriteLine("No orders found.");
}
}
}
Introducing a safety net with null checks transforms the code into a reliable module:
public class OrderProcessor
{
public void GetCustomerOrderDetails(Customer customer)
{
if (customer == null)
{
Console.WriteLine("Customer is null.");
return;
}
if (customer.Orders == null)
{
Console.WriteLine("Customer has no orders.");
return;
}
if (customer.Orders.Count > 0)
{
foreach (var order in customer.Orders)
{
Console.WriteLine($"Order ID: {order.OrderId}, Amount: {order.Amount}");
}
}
else
{
Console.WriteLine("No orders found.");
}
}
}
Unearthing Anti-Patterns
Ponder on the following code snippet and recognize the pitfalls of tight coupling.
public class OrderService
{
private readonly PaymentService _paymentService;
public OrderService()
{
_paymentService = new PaymentService();
}
public void ProcessOrder(Order order)
{
_paymentService.ProcessPayment(order);
}
}
Decoupling enhances testability and flexibility:
public class OrderService
{
private readonly IPaymentService _paymentService;
public OrderService(IPaymentService paymentService)
{
_paymentService = paymentService;
}
public void ProcessOrder(Order order)
{
_paymentService.ProcessPayment(order);
}
}
Banish Magic Numbers
As you review, watch for unnamed constants that obscure intent and complicate maintenance:
public double CalculateTax(double income)
{
if (income <= 50000)
return income * 0.10;
if (income <= 100000)
return income * 0.15;
if (income <= 200000)
return income * 0.20;
return income * 0.30;
}
Confine such magic to configurations, refining accuracy with simplicity:
public double CalculateTax(double income, IConfiguration configuration)
{
double[] incomeSlabs = configuration.GetSection("TaxSettings:IncomeSlabs").Get<double[]>();
double[] taxRates = configuration.GetSection("TaxSettings:TaxRates").Get<double[]>();
for (int i = 0; i < incomeSlabs.Length; i++)
{
if (income <= incomeSlabs[i])
{
return income * taxRates[i];
}
}
return income * taxRates.Last();
}
{
"TaxSettings": {
"IncomeSlabs": [50000, 100000, 200000],
"TaxRates": [0.10, 0.15, 0.20, 0.30]
}
}
Upholding the DRY Principle
Repetitive logic dulls innovation, yet clarity emerges with consolidation.
public class DiscountCalculator
{
public double CalculateDiscount(double amount, double discountPercentage)
{
double discount = amount * discountPercentage;
double discountedPrice = amount - discount;
return discountedPrice;
}
public double ApplyDiscount(double amount, double discountPercentage)
{
double discount = amount * discountPercentage;
double discountedPrice = amount - discount;
return discountedPrice;
}
}
A refactored design champions simplicity:
public class DiscountCalculator
{
public double CalculateDiscount(double amount, double discountPercentage)
{
return CalculateDiscountAmount(amount, discountPercentage);
}
public double ApplyDiscount(double amount, double discountPercentage)
{
double discount = CalculateDiscountAmount(amount, discountPercentage);
return amount - discount;
}
private double CalculateDiscountAmount(double amount, double discountPercentage)
{
return amount * discountPercentage;
}
}
Embracing YAGNI
Envision clarity by challenging preemptive complexities, rooted in the absence of necessity.
public class OrderProcessor
{
public void ProcessOrder(Order order, double discount)
{
decimal discountAmount = CalculateDiscount(order, discount);
decimal finalAmount = order.Amount - discountAmount;
Console.WriteLine($"Discount Applied: {discountAmount:C}");
if (!string.IsNullOrEmpty(order.CouponCode))
{
decimal couponDiscount = ApplyCoupon(order.CouponCodeDiscount);
finalAmount -= couponDiscount;
Console.WriteLine($"Coupon {order.CouponCode} applied.");
}
}
private decimal CalculateDiscount(Order order, double discount)
{
return order.Amount * discount;
}
private decimal ApplyCoupon(double couponCodeDiscount)
{
return order.Amount * couponCodeDiscount;
}
}
public class Order
{
public int Id { get; set; }
public decimal Amount { get; set; }
public double CouponCodeDiscount { get; set; }
}
Discard unnecessary additions to maintain focus:
public class OrderProcessor
{
public void ProcessOrder(Order order, double discount)
{
decimal discountAmount = CalculateDiscount(order, discount);
decimal finalAmount = order.Amount - discountAmount;
Console.WriteLine($"Discount Applied: {discountAmount:C}");
}
private decimal CalculateDiscount(Order order, double discount)
{
return order.Amount * discount;
}
}
public class Order
{
public int Id { get; set; }
public decimal Amount { get; set; }
}
Maintaining Professionalism in Code Reviews
Elevate the collaborative spirit of your team by embodying professionalism and respect:
- Respectful Interactions: Improve the code, not the individual.
- Constructive Feedback: Guide through suggestions, not criticisms.
- Inquisitive Dialogues: Ignite learning through thoughtful queries.
- Clear Precision: Articulate specific feedback for better understanding.
- Balanced Evaluation: Celebrate strengths as you address weaknesses.
- Open-Mindedness: Welcome diverse viewpoints to enrich outcomes.
- Patient Guidance: Mentor with kindness, fostering growth.
- Avoid Nitpicking: Prioritize impactful feedback over minutiae.
- Manage Bias: Focus beyond personal preferences.
- Value Time: Share considerate and concise insights promptly.
Adopting these practices nurtures a vibrant and inspired code-review culture. Bring your insights to the conversation: what strategies define your review approach?
