Your code should be clear enough and easy enough to read that you don't need comments.
You should be naming your methods and variables so clearly that it is obvious what is happening in your code.
The exception to this would be when the code is doing something out of the ordinary and you need to add an explanation. If you keep your methods small enough you can put the comments in the summary above the method.
The problem with comments is that when the code gets updated if you don't update the comments, they will be wrong.
The same can be said about summaries above the method or class. They can very quickly go out of date.
Watch my video about this topic
Consider these two examples
This one is busy with unnecessary comments and magic numbers:
static string GetPrettyDate(DateTime d)
{
// 1.
// Get time span elapsed since the date.
TimeSpan s = DateTime.Now.Subtract(d);
// 2.
// Get total number of days elapsed.
int dayDiff = (int)s.TotalDays;
// 3.
// Get total number of seconds elapsed.
int secDiff = (int)s.TotalSeconds;
// 4.
// Don't allow out of range values.
if (dayDiff < 0 || dayDiff >= 31)
{
return null;
}
// 5.
// Handle same-day times.
if (dayDiff == 0)
{
// A.
// Less than one minute ago.
if (secDiff < 60)
{
return "just now";
}
// B.
// Less than 2 minutes ago.
if (secDiff < 120)
{
return "1 minute ago";
}
// C.
// Less than one hour ago.
if (secDiff < 3600)
{
return string.Format("{0} minutes ago", Math.Floor((double)secDiff / 60));
}
// D.
// Less than 2 hours ago.
if (secDiff < 7200)
{
return "1 hour ago";
}
// E.
// Less than one day ago.
if (secDiff < 86400)
{
return string.Format("{0} hours ago", Math.Floor((double)secDiff / 3600));
}
}
// 6.
// Handle previous days.
if (dayDiff == 1)
{
return "yesterday";
}
if (dayDiff < 7)
{
return string.Format("{0} days ago",
dayDiff);
}
if (dayDiff < 31)
{
return string.Format("{0} weeks ago",
Math.Ceiling((double)dayDiff / 7));
}
return null;
}
This one just has a summary at the top, and uses clearly defined variables and constants instead of magic numbers.
/// <summary>
/// Returns a string representing the relative time for when the tweet was tweeted.
/// I started out using this as a base, but rewrote some of it: http://www.dotnetperls.com/pretty-date
/// </summary>
/// <param name="startDate">The start date</param>
/// <param name="endDate">The end date</param>
/// <returns>A string representing the relative time between the two dates</returns>
public static string GetRelativeTimeBetweenDates(DateTime startDate, DateTime endDate)
{
const int MAXIMUM_DAYS_IN_MONTH = 31;
const int ONE_MINUTE = 60;
const int TWO_MINUTES = 120;
const int ONE_HOUR = 3600;
const int TWO_HOURS = 7200;
const int ONE_DAY = 86400;
const int ONE_WEEK = 7;
const string DATE_FORMAT = "dd mmm yyyy";
TimeSpan interval = endDate.Subtract(startDate);
int differenceInDays = (int)interval.TotalDays;
int differenceInSeconds = (int)interval.TotalSeconds;
string relativeTime = "";
if (differenceInDays >= 0 && differenceInDays < MAXIMUM_DAYS_IN_MONTH)
{
if (differenceInDays == 0)
{
if (differenceInSeconds < ONE_MINUTE)
{
relativeTime = "just now";
}
else if (differenceInSeconds < TWO_MINUTES)
{
relativeTime = "1 minute ago";
}
else if (differenceInSeconds < ONE_HOUR)
{
relativeTime = string.Format("{0} minutes ago", Math.Floor((double)differenceInSeconds / ONE_MINUTE));
}
else if (differenceInSeconds < TWO_HOURS)
{
relativeTime = "1 hour ago";
}
else if (differenceInSeconds < ONE_DAY)
{
relativeTime = string.Format("{0} hours ago", Math.Floor((double)differenceInSeconds / ONE_HOUR));
}
}
else if (differenceInDays == 1)
{
DateTime yesterday = endDate.AddDays(-1);
if (startDate.Date == yesterday.Date)
{
relativeTime = "yesterday";
}
else
{
relativeTime = "2 days ago";
}
}
else if (differenceInDays < ONE_WEEK)
{
relativeTime = string.Format("{0} days ago", differenceInDays);
}
else if (differenceInDays < MAXIMUM_DAYS_IN_MONTH)
{
double numberOfWeeks = Math.Ceiling((double)differenceInDays / ONE_WEEK);
relativeTime = string.Format("{0} week{1} ago", numberOfWeeks, numberOfWeeks > 1 ? "s" : "");
}
else
{
DateTime oneMonthAgo = endDate.AddMonths(-1);
if (startDate.Year == oneMonthAgo.Year && startDate.Month == oneMonthAgo.Month)
{
relativeTime = "last month";
}
else
{
relativeTime = startDate.ToString(DATE_FORMAT);
}
}
}
return relativeTime;
}
I believe the second example is a lot clearer and easier to understand.
What are your thoughts on the topic? Can you offer a good case for using comments?