Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

Honestly, there's very little code where comments on the function calls can't be helpful. Let's use GetUserDetails(userId)

A comment like "gets the details about a user" is of course a waste of time.

What I like to see is everything about using it, without having to read the code. For example: "Gets details for a user, based on the Service.CurrentUser access rights. If userId doesn't exist, is not available valid id, or the current user diesn't have permission, throws exception. Note this can return data for disabled and deleted users, so if used for non-admin functionality, you should check the State property. Due to caching, this might not show changes made in another session in the past couple minutes."

The lack of a check for disabled status (for example) won't be obvious if I'm not actively thinking about checking disabled status. That few seconds of a comment can easily prevent a bug when someone consumes this - especially if they didn't write this API or are new to the cosebase (maybe they don't even know disabled users are a thing).

Sometimes when you write these types of comments you realize your naming is bad, or that there's an API change that would make it easier to avoid a mistake - such as having separate GetActiveUserDetails() and AdminGetUserDetails().

I think about this during vice reviews, too: I've often asked for comments to be added on existing methods being consumed (but not modified in the code I'm reviewing) because the new use made me wonder about how something would be handled and I had to go read a couple levels deep to figure it out.



Agree. And in a richly-typed language you can be more specific:

    getUserDetails :: UserID -> DBAction (Maybe AnyUser)
This type says a lot already. The definition of `AnyUser` would specify the parameters that the comment only says in prose. Or we could be more specific

    getActiveUserDetails :: UserID -> DBAction (Maybe ActiveUser)
We could also include the authorization specification in the type constraints on the `DBAction`:

    getUserDetails :: (DBAction m, HasAdminContext m) => UserID -> m (Maybe AnyUser)
This is what I mean when I say that a rich type system can enforce a lot of what you might traditionally use comments for in a weaker type system. Now my hypothetical comment might highlight some salient points about this function, such as performs N queries to validate all of the available authorization contexts, use with a user role that has a limited number of contexts for best performance.




Consider applying for YC's Fall 2025 batch! Applications are open till Aug 4

Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: