I'm not confident that I conveyed the point I wanted to, but hopefully I did. I encourage discussion on this topic as it is a relatively vague and abstract topic that needs discussion.
You forgot about useful rule: name arguments in prototypes too.
article wrote:
void drawRectangle(Point, Dimension);
[...]
The second takes a point and a dimension, and a rectangle can be drawn with just a point, a width, and a height. It's still slightly ambiguous though, as we don't know if the given point is the center or the top-left corner,
We're sort of on the Hungarian wagon a bit, too. Not fully and not religiously. Don't really use it for data types (I guess it could become a pain if they change) other than pointers. We do use the m_ prefix for members, though, and the other odd feature, such as a k prefix for (non-local) constants.
We use javadoc style comments to document our functions. There is many things you cannot tell though the name
For example: your DrawRectangleFromOrigin. What color will that rectangle be? Will it be filled? Does it depend on global state (like PenColor/FillStyle)? Where will it be drawn? What is width? Percentage of drawing surface width? Pixels? Centimeters? Same for origin.
Some reference-style comment would be nice:
1 2 3 4 5 6
//Draws a rectange on drawing surface specified by global variable CurrentDrawingContext
//Color and style depends on DrawingTools of drawing surface
//@origin: distance from top-left corner in pixels
//@width, height: width and height in pixels
//@return: none
//@throw: IllegalArgument if either width or height is 0 or any pixel of resulting rectangle is outside drawing surface
Some of it is redundant, but it is really good to automaticly make a reference for your library.
Some other comments I think were needed:
1 2 3 4 5 6 7
//A single shaker sort pass: Leaderboard cannot change drastically in 1 frame
//so it is enough. O(n) FTW.
//Bit hack to place 4 floats in 128 bit variable
//DO NOT change to function call unless you want extreme slowdown.
//Balances bug in library we cannot change. Do not "fix"
I think the reason iHutch's workplace discourages comments is for these three reasons:
- it slows down development time to write comments
- it slows down development time to read comments
- if the comment is non-trivial, the code is poorly written
@xander: I'm still learning CSS, do you have a recommendation about how I can change my light.css? I'm considering something along the lines of making the body tag have a width of 75%
EDIT: I changed the left and right padding to 10% each, does that look better?
Because the code is much less polluted that way. We only really use them where necessary.
The rectangle example was exactly that, an example, but it could be expanded. Colour and whether it's filled could well be easily described arguments (vec4 colour, bool isFilled). My point isn't the method itself or the number of arguments; I was trying to highlight the naming style.
I should point out that we're a pretty small team (8) of engineers working on a very specific project. I guess on a bigger scale, comments would be slightly more necessary. I previously worked for a company that (as of 2011) had 3.5 millions lines of comments in their system (yuck).
Your second comment example is fair enough and present in what we do. We tend to comment when the code looks slightly odd. The mantra is that you should be able to look at the code itself and understand what it's doing.
The first comment example is hideous, though. Most of that information can be easily inferred and some of it is just plain unnecessary. Again, maybe less so on a bigger scale.
Edit: Quite right, LB. In particular the third point. Also, the padding does make the article a little easier on the eyes. :-)
If I may suggest something - I would change background to be dark. Maybe it's just me, but it's much much easier for me to read bright or not-too-dark text from dark background(deep blue or black, for example), than from light one.
@MatthewRock: If you're using Chrome, press F12 to open the dev console and in the HTML head tag, edit the stylesheet from light.css to dark.css and tell me what you think. I'm personally not a fan of light-on-dark, but I'm also a terrible artist and suck at colors.
I get it, I've seem similar things, like Doxygen. I still think it's overkill, though. Of the information there, the functionality and arguments could be inferred by name. The return type is present in the declaration and implementation.
The only thing that might be of use is the throw. However, I'd expect to be able to see that with reasonable ease within the function, keeping in mind that the function itself should be reasonably sized.
Exception handling is still a touchy subject for me, I can't make up my mind on it. As such, I have no idea how you would write self-documenting code with exceptions - maybe something with the comma operator and noexcept(false).
The example is too simple.
Here is standard library one:
1 2
template< class InputIt, class UnaryPredicate >
InputIt find_if( InputIt first, InputIt last, UnaryPredicate p );
What is returned if nothing is found? What constraints are placed on p? Can it be statefull functor? Will predicate be applied consequentally? Can we be sure that it will not change in future?
I am not saying that it should be used by everyone. But if you are writing generic library which will be heavely reused, you should consider it: it saves time writing documentation, it is less likely that someone forge to update comment right before function that some text file in the depth of project, you can check if doc was updated properly in commit history.
Back to the article discussion. One problem with using aliases to classes which currenly works for you, but actually have a wider range, is that once you might notice that there is Suit_t::npos used everywere. So to lessen chance for that to happen, you should document possible usage of your class (i.e. Suit_t can be compared using equal and not equal operator, can be assigned values from SUITS container and can be assigned to other Suit_t)
Consistent with the rest of the C++ standard library, last is returned. Since this is consistent it only has to be learned once and recognized everywhere.
MiiNiPaa wrote:
What constraints are placed on p?
The constraints of a UnaryPredicate? It's described once and consistent everywhere.
MiiNiPaa wrote:
Can it be statefull functor?
Considering how it is passed to the function, yes.
MiiNiPaa wrote:
Will predicate be applied consequentally?
?
MiiNiPaa wrote:
Can we be sure that it will not change in future?
?
MiiNiPaa wrote:
Back to the article discussion. One problem with using aliases to classes which currenly works for you, but actually have a wider range, is that once you might notice that there is Suit_t::npos used everywere. So to lessen chance for that to happen, you should document possible usage of your class (i.e. Suit_t can be compared using equal and not equal operator, can be assigned values from SUITS container and can be assigned to other Suit_t)
Did you read only the odd-numbered sentences or something?
I have no idea how you would write self-documenting code with exceptions
1 2 3 4 5
double arcsin(double value)
{
if (value < -1 || value > 1) //1st layer of self-documentation
throw domain_error(format(
"arcsin defined in range [-1;1] value passed was: %1", value) ) //2nd layer
I don't like the idea of having to make assumptions about the behavior and use of something. Everything should have a clear unambiguous documentation that defines the use and behavior of a module and that definition itself should ideally be modular. You should not need to go searching around to put all of the pieces together if you haven't memorized everything, and you should not be required make any assumptions. Everything you need to know about the use should be in the form of a guarantee.
Did you read only the odd-numbered sentences or something?
I do not know how I managed to miss tat sentence.
Still I prefer to say that self-documenting code should only have one right way to use it. Simple adapter which allow you to use only allowed methods will save you from someone who decide to use non-documented features of your type.
Consistent with the rest of the C++ standard library
It was an example.
Actually it was one point I wanted to say: consistency across your code and basically usage of Concepts will make your code cleanier and easier to use.
But still there will be cases when you cannot be sure on constraints on arguments and possible return values. Can your function returning double return a NaN? Or infinity? Can destination be the same as beginning? And other similar things. Like whole C library.
Considering how it is passed to the function, yes.
Scott Meyers thinks that you should not use stateful functors with standard library algorithms.
?
Will that work?
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15
class functor
{
int x;
public:
functor() : x(0) {}
booloperator()(int x)
{
if (x < 0)
++x;
if (x == 3)
returntrue;
returnfalse;
}
//return third negative value:
auto x = find_if( begin, end, functor());