Code Smell - Too Many Comments
On the surface, comments sound helpful, and the more, the better. It shows that an Engineer is considering the next Engineer that will be maintaining it and taking pride in documenting their work.
But while intentions are good, comments can be lies waiting to happen. For example, will the next Engineer remember to update the comment when they change the code?
A better approach is self-documenting code. Try to write the code, so it's easier to read and understand, removing the need for comments. However, even though developers can understand how a particular code works, they might not understand why it was implemented in that manner. Comments can be helpful here to explain why something was done a certain way.
Here's an example of a C# Student class with too many comments:
public class Student {
private string name;
private List<int> grades;
public Student(string name) {
this.name = name;
this.grades = new List<int>();
}
// This method adds a new grade to the student's record. If the grade is negative or exceeds the maximum allowed
// grade value, an ArgumentException is thrown with a descriptive message. Otherwise, the grade is added to the
// internal list of grades.
public void AddGrade(int grade) {
if (grade < 0 || grade > 100) {
throw new ArgumentException("Invalid grade value. Grade must be between 0 and 100 inclusive.");
}
grades.Add(grade);
}
// This method calculates the average grade for the student. If the student has not yet received any grades,
// a InvalidOperationException is thrown with a descriptive message. Otherwise, the average is calculated by
// summing up all the grades and dividing by the number of grades, and the result is returned.
public double GetAverageGrade() {
if (grades.Count == 0) {
throw new InvalidOperationException("Cannot calculate average grade. Student has not received any grades.");
}
int totalGrade = 0;
foreach (int grade in grades) {
totalGrade += grade;
}
return (double) totalGrade / grades.Count;
}
// This method returns a string representation of the student, including their name and their list of grades.
public override string ToString() {
StringBuilder sb = new StringBuilder();
sb.Append(name).Append(": ");
foreach (int grade in grades) {
sb.Append(grade).Append(", ");
}
if (grades.Count > 0) {
sb.Remove(sb.Length - 2, 2);
}
return sb.ToString();
}
}
In this example, the comments on the methods are redundant and do not provide any additional information that is not already clear from the method names and parameters.
public class Student {
private string name;
private List<int> grades;
public Student(string name) {
this.name = name;
this.grades = new List<int>();
}
public void AddGrade(int grade) {
if (grade < 0 || grade > 100) {
throw new ArgumentException("Invalid grade value. Grade must be between 0 and 100 inclusive.");
}
grades.Add(grade);
}
public double GetAverageGrade() {
if (grades.Count == 0) {
throw new InvalidOperationException("Cannot calculate average grade. Student has not received any grades.");
}
return grades.Average();
}
public override string ToString() {
return $"{name}: {string.Join(", ", grades)}";
}
}
This implementation is simpler and more readable without sacrificing clarity or functionality.
This is an example where the comments can be useful in describing why something was done in that way:
/**
* Calculates the total revenue for a given product over a specified date range.
*
* The method first retrieves all sales data for the product from the database, filtered by
* the date range provided. It then calculates the total revenue by summing the sale price of
* each item sold. Note that this method does not take into account any discounts or other
* promotions that may have been applied to the sale.
*
* The use of LINQ allows for efficient filtering and grouping of the sales data, and reduces
* the amount of code needed to perform the calculation. Additionally, the use of a strongly-typed
* result object makes it easy to work with the results of the calculation.
*/
public ProductRevenue CalculateProductRevenue(int productId, DateTime startDate, DateTime endDate)
throws DatabaseException {
var salesData = database.GetSalesDataForProduct(productId)
.Where(sale => sale.Date >= startDate && sale.Date <= endDate);
var revenueData = from sale in salesData
group sale by 1 into g
select new
{
TotalRevenue = g.Sum(sale => sale.Price),
NumItemsSold = g.Count()
};
var result = new ProductRevenue(revenueData.First().TotalRevenue, revenueData.First().NumItemsSold);
return result;
}