Code Smell — Practical Guide using .Net Core Part-II
Code Smell — Feature Envy, Inconsistent Naming, Too Many Comments and Switch Statements
In previous story Code Smell — Practical Guide using .Net Core Part-I we talked about about the Code Smell, identification and remedies and enlisted the most prevalent code smells
In this story we'll cover Feature Envy, Inconsistent Naming, Too Many Comments and Switch Statements
5. Feature Envy
Feature Envy is a code smell that occurs when a method seems more interested in the data of another class than in its own class. This often leads to increased coupling between classes and can make the code less maintainable. In .NET Core, you might encounter Feature Envy in your C# code. Here’s an example:
public class Car
{
private string model;
private Engine engine;
public Car(string model, Engine engine)
{
this.model = model;
this.engine = engine;
}
// Feature Envy smell
public bool IsHighPerformance()
{
// This method is more interested in the Engine's properties
// than the Car's properties
return engine.Horsepower > 300 && engine.FuelType == FuelType.Premium;
}
}
public class Engine
{
public int Horsepower { get; set; }
public FuelType FuelType { get; set; }
}
public enum FuelType
{
Regular,
Premium
}In the example above, the IsHighPerformance method in the Car class is more interested in the properties of the Engine class (which is a different class) than in the properties of the Car itself. This is a potential code smell, and it might be more appropriate for this logic to be part of the Engine class or in a separate class that is responsible for evaluating engine performance.
To refactor the code and address the Feature Envy smell, we might consider moving the relevant logic into the Engine class:
public class Car
{
private string model;
private Engine engine;
public Car(string model, Engine engine)
{
this.model = model;
this.engine = engine;
}
}
public class Engine
{
public int Horsepower { get; set; }
public FuelType FuelType { get; set; }
// Move the relevant logic here
public bool IsHighPerformance()
{
return Horsepower > 300 && FuelType == FuelType.Premium;
}
}
public enum FuelType
{
Regular,
Premium
}By doing this, we reduce the Feature Envy and make the code more aligned with the Single Responsibility Principle, as each class is responsible for its own logic.
6. Inconsistent Casing/Naming
Inconsistent naming is a code smell that can make your code harder to understand and maintain. Adopting a consistent and meaningful naming convention is crucial for the clarity and readability of your code. Here are some examples of inconsistent naming and how to address them in a .NET Core context:
- Inconsistent Casing:
- Symptom: Using different casings (e.g., camelCase, PascalCase) for variables, methods, or class names inconsistently.
- Solution: Choose a consistent casing convention and apply it uniformly throughout your codebase.
// Inconsistent casing
int myVariable;
string MyMethod() { /* ... */ }
// Consistent casing (PascalCase)
int MyVariable;
string MyMethod() { /* ... */ }2. Abbreviations and Acronyms:
- Symptom: Inconsistent use of abbreviations or acronyms in names.
- Solution: Choose whether to use abbreviations or spell out words fully and be consistent across the codebase.
// Inconsistent use of abbreviation
int numOfUsers;
int numberOfEmployees;
// Consistent use of abbreviation
int numUsers;
int numEmployees;3. Verbosity:
- Symptom: Some methods or variables are overly verbose, while others use short or unclear names.
- Solution: Strike a balance between clarity and brevity. Use descriptive names that convey the purpose without being excessively long.
// Inconsistent verbosity
int CalculateSum(int a, int b) { /* ... */ }
int Calc(int x, int y) { /* ... */ }
// Consistent verbosity
int CalculateSum(int operand1, int operand2) { /* ... */ }
int Calculate(int num1, int num2) { /* ... */ }4. Naming Conventions:
- Symptom: Violating established naming conventions within the codebase.
- Solution: Ensure that all developers follow the same naming conventions. This is especially important in a team setting.
/// Inconsistent naming conventions
int my_variable;
string MyMethod() { /* ... */ }
// Consistent naming conventions (camelCase)
int myVariable;
string myMethod() { /* ... */ }By addressing inconsistent naming, you contribute to the overall maintainability and readability of your .NET Core codebase, making it easier for developers to understand and collaborate on the project.
7. Too Many Comments
The “Too Many Comments” code smell refers to situations where the code contains an excessive number of comments. While comments can be helpful for explaining complex logic or providing context, an overreliance on comments may indicate that the code is not self-explanatory. Here are some considerations and solutions for addressing the “Too Many Comments” code smell in .NET Core:
- Clear and Concise Code:
Write code that is clear and concise, reducing the need for excessive comments. Well-named variables, functions, and classes can go a long way in making the code self-explanatory. - Self-Documenting Code:
Aim for self-documenting code that expresses its intent through its structure and naming conventions. Code should ideally be easy to understand without the need for extensive comments. - Outdated Comments:
Comments can become outdated if the code changes but the comments aren’t updated accordingly. Outdated comments can be more harmful than helpful. Regularly review and update comments to ensure accuracy.
Solutions:
- Refactor Complex Code:
If you find yourself adding comments to explain overly complex logic, consider refactoring the code to simplify it. Break down large methods into smaller, more manageable ones.
// Smelly code with excessive comments
// Calculate the total price after applying discounts
// and taxes to each item in the shopping cart.
public decimal CalculateTotalPrice(List<CartItem> cartItems)
{
// ... complex logic ...
}
// Better approach with simplified code
public decimal CalculateTotalPrice(List<CartItem> cartItems)
{
ApplyDiscounts(cartItems);
ApplyTaxes(cartItems);
return CalculateSum(cartItems);
}2. Remove Redundant Comments:
Eliminate comments that state the obvious or provide information that is already evident from the code itself. Redundant comments can clutter the code and reduce readability.
// Redundant comment
int result = x + y; // Add x and y
// Improved code without redundant comment
int result = x + y;3. Use Inline Comments Sparingly:
Instead of long paragraphs of comments, use concise inline comments only when necessary. Focus on explaining why something is done, not just what the code is doing.
// Inline comment explaining the purpose
if (condition) // Check if the condition is met
{
// ... code ...
}4. Documentation and READMEs:
Consider moving non-essential information, such as design decisions or architecture explanations, to project documentation or README files. This helps maintain a clean codebase while providing necessary context.
By addressing the “Too Many Comments” code smell, we can improve the maintainability and readability of your .NET Core code. Always strive for a balance between clarity in code and the use of comments where they add genuine value.
8. Switch Statements
A switch statement is not inherently a code smell, but the way it’s used in certain scenarios can lead to code smells. One common code smell associated with switch statements is the violation of the Open/Closed Principle, which is one of the SOLID principles. The Open/Closed Principle states that a class should be open for extension but closed for modification.
When you find yourself adding new cases to a switch statement frequently, it might be an indication that your code is not easily extensible. Each time you add a new case, you have to modify the existing switch statement, potentially introducing bugs and making your code harder to maintain.
Here’s an example of a code smell associated with switch statements:
public class PaymentProcessor
{
public void ProcessPayment(PaymentType paymentType, decimal amount)
{
switch (paymentType)
{
case PaymentType.CreditCard:
ProcessCreditCardPayment(amount);
break;
case PaymentType.PayPal:
ProcessPayPalPayment(amount);
break;
case PaymentType.Bitcoin:
ProcessBitcoinPayment(amount);
break;
// More cases might be added in the future
// ...
default:
throw new ArgumentException("Invalid payment type", nameof(paymentType));
}
}
private void ProcessCreditCardPayment(decimal amount)
{
// Process credit card payment logic
}
private void ProcessPayPalPayment(decimal amount)
{
// Process PayPal payment logic
}
private void ProcessBitcoinPayment(decimal amount)
{
// Process Bitcoin payment logic
}
}
public enum PaymentType
{
CreditCard,
PayPal,
Bitcoin
// More payment types might be added in the future
// ...
}In this example, if we need to add a new payment type, we have to modify the ProcessPayment method by adding a new case. This violates the Open/Closed Principle.
A better approach would be to use polymorphism and create a strategy pattern. Each payment type can have its own class implementing a common interface, and the PaymentProcessor can delegate the payment processing to the corresponding strategy. Here is an example:
public interface IPaymentProcessor
{
void ProcessPayment(decimal amount);
}
public class CreditCardPaymentProcessor : IPaymentProcessor
{
public void ProcessPayment(decimal amount)
{
// Process credit card payment logic
}
}
public class PayPalPaymentProcessor : IPaymentProcessor
{
public void ProcessPayment(decimal amount)
{
// Process PayPal payment logic
}
}
// Similar classes for other payment types
public class PaymentProcessor
{
private readonly Dictionary<PaymentType, IPaymentProcessor> paymentProcessors;
public PaymentProcessor()
{
paymentProcessors = new Dictionary<PaymentType, IPaymentProcessor>
{
{ PaymentType.CreditCard, new CreditCardPaymentProcessor() },
{ PaymentType.PayPal, new PayPalPaymentProcessor() }
// Add more payment processors for other types
};
}
public void ProcessPayment(PaymentType paymentType, decimal amount)
{
if (paymentProcessors.TryGetValue(paymentType, out var processor))
{
processor.ProcessPayment(amount);
}
else
{
throw new ArgumentException("Invalid payment type", nameof(paymentType));
}
}
}In this refactored example, the PaymentProcessor no longer needs to be modified when adding new payment types, and each payment type has its own class responsible for processing payments. This adheres more closely to the Open/Closed Principle.
In this story, we delved into the manifestations of code smells such as Feature Envy, Inconsistent Naming, Too Many Comments and Switch Statements. The subsequent segment will encompass the remaining ones as a part of this story.
Conclusion
This story is a part of three parts as
Code Smell — Practical Guide using .Net Core Part-I
Code Smell — Practical Guide using .Net Core Part-II
Code Smell — Practical Guide using .Net Core Part-III
