1

I've been working on a ASP.NET Web Api 2 project as a learning exercise, and I was wondering what the norm regarding commenting is.

I use the Repository / Service Pattern, and everything is very much decoupled, which results in having a large number of classes whose purpose is pretty clear. I also name my variables in a way that they explain themselves.

This leaves me wondering what I should actually be commenting in this scenario.

  • Should I just stick to class-summaries ( even if the class itself is self-explanatory ) ?
  • What about self-explanatory methods ?
  • Do data / business entity classes need a summary ?
  • What about interfaces ?

To give a bit more context, here is one of my repository classes which provides CRUD functionality:

public class UserRepository : RepositoryBase<User, decimal>
{
    private readonly ShortStuffContext _context;

    public UserRepository(ShortStuffContext context)
    {
        _context = context;
    }

    public override IEnumerable<User> GetAll()
    {
        return _context.Users.BuildUser();
    }

    public override User GetById(decimal id)
    {
        return _context.Users.FirstOrDefault(u => u.Id == id).BuildUser();
    }

    public override CreateStatus<decimal> Create(User entity)
    {
        if (_context.Users.Any(u => u.Id == entity.Id))
        {
            return new CreateStatus<decimal> { status = CreateStatusEnum.Conflict };
        }

        var user = new Data.Entities.User();
        user.InjectFrom<SmartConventionInjection>(entity);
        _context.Users.Add(user);
        _context.SaveChanges();

        return new CreateStatus<decimal> { status = CreateStatusEnum.Created, Id = user.Id };
    }

    public override UpdateStatus Update(User entity)
    {
        var dbUser = _context.Users.FirstOrDefault(u => u.Id == entity.Id)
                             .InjectFrom<NotNullInjection>(entity);

        if (dbUser != null)
        {
            var changeTrackerUser = _context.ChangeTracker.Entries<Data.Entities.User>()
                            .FirstOrDefault(u => u.Entity.Id == entity.Id);

            changeTrackerUser.CurrentValues.SetValues(dbUser);
            if (_context.SaveChanges() == 0)
            {
                return UpdateStatus.NoChange;
            }
            return UpdateStatus.Updated;
        }
        return UpdateStatus.NotFound;
    }

    public override void Delete(decimal id)
    {
        var user = _context.Users.FirstOrDefault(u => u.Id == id);
        _context.Users.Remove(user);
        _context.SaveChanges();
    }
}

So in a class like that, is it worthwhile to put a summary like "Provides CRUD functionality for the User Data Entity" ( and then repeat that summary for every other repository ), or not ? What about method comments, or inline comments?

Edit: My project is on Github. I know about adding comments to more complex methods, and I'm planning on adding comments to the Extensions and the ValueInjecter classes soon ( which hold by far the most complex code ). Apart from that, any advice regarding where to put comments is very much welcome.

  • 1
    If I were looking for comments I'd note that I don't know what GetUserById returns if a user with a given id is not found (Null, default or throws an exception) – Kevin Jul 5 '14 at 4:06
  • If comments are going to be written at all (e.g. Why a particular design was chosen over alternatives), they belong in your commit messages to source control; by doing that, your comments are automatically synchronized with latest source (they become metadata of the source history) As to adorning code itself with comments, if you feel you need to write a comment to clarify the meaning of your code, then you need to stop writing unclear code. – K. Alan Bates Jul 11 '15 at 3:19
3

There is no definitive answer: some people will be inclined to add comments to every class and method; others will try to avoid commenting things which are already explicit enough.

  • If you follow StyleCop default rules, you have to comment every member, including private fields. You might want to suppress the warnings if the code is really repetitive. But if it is, you may ask yourself why is it not automatically generated (note: automatically generated code is marked specifically, and is not processed by StyleCop).

  • If you don't, feel free to comment as much or little as you feel appropriate. If I were maintaining the piece of code from your question, I would have added comments on every method, just to simplify my work later: I'd rather read a sentence in a tooltip in Visual Studio rather than try to guess right the meaning of the method. On the other hand, I would avoid commenting something like Title property of the class Product.

In all cases, try to bring something useful on those comments, not just repeat the name of the method or property. For example, if the class Product has a property Description, one way is to write:

/// <summary>
/// Gets the description of the product.
/// </summary>

This comment doesn't bring anything useful. Another possibility is this one:

/// <summary>
/// Gets the piece of text which is displayed publicly and is usually written by the
/// managers. The description is localized for the current user.
/// </summary>

Here, you bring three pieces of useful information:

  • The description is public. The web app may also have an internal summary field where managers can write text for themselves.

  • The description is usually written by managers. This helps understanding the business logic.

  • The description is a string translated in the language of the current user (probably CultureInfo.CurrentCulture).

  • You bring up a good point about Visual Studio Tooltips, which I had not considered. While a lot of the comments on methods may be fairly obvious when looking at the method, they would still be useful for the tooltip. I don't know about StyleCop, but I will read up about it. Thank you. – Bio2hazard Jul 6 '14 at 22:36
2

If you feel that you can't add any useful information to the future reader of this classes in comments don't write it. In my opinion your code don't need comments, its pretty obvious, comments in this code are more noise than real information.

A programming language its of course a language, when you put a comment is because you can't express the things you want to express in this programming language and you use another language that gives you more freedom and tools for express (a human language like english). Sometimes you can justify this because programing language can have some obscure features or ways to do things that need an explanation, or perhaps you use some third party library with a very strange behavior than need an explantation. But IMMO the majority of the code don't need the use of a human language to express things, a lot of times the comments reflect a bad design, bad name choices, bad architecture etc,etc.

Every time when i think "i need a comment here for others to understand this" an alert its raised and i try to rethink my design, search for better names, do some refactors etc, etc, sometimes this helps me to find a better solution and i don't need the comment, sometimes i don't find a better solution an i put the comment, but i never put a comment before making a effort to try to express things more clearly in my programming language.

0

I write comments for two purposes

  1. Class / Method XML comments. To provide context to what a method/class/snippet does and why it may be there. This can 'repeat' what the code clearly expresses, but only to the person who wrote it. If the comment can tell me what is occurring without understanding the code it is a benefit
  2. To highlight edge cases. In the event where I need to handle a specific case I'll leave an inline comment.

To expand on my comment:

If I were looking for comments I'd note that I don't know what GetUserById returns if a user with a given id is not found (Null, default or throws an exception).

Simiarly I would leave a comment on create status saying that a Status of Conflict is returned when the entity's id matches an existing user id.

Comments may seem like they're regurgitating code, but this is only to those who read and understand it. Consider that you need not do this in order to use it.

protected by gnat May 30 '18 at 11:36

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

Not the answer you're looking for? Browse other questions tagged or ask your own question.