Hello everyone,
Not an altogether important question but one I have been wandering about for some time. Say we have the following example.
Point(int x, int y){
this->x=x;
this->y=y;
}
I don't see this very often, and I have heard, and read some snarky remarks with respect to the above code. Just wandering if there are any problems which I don't understand.
Bad form/style by 2016 standards?
Performance hit?
Too verbose?
Thanks,
Mike
There are circumstances where you may need to use it to disambiguate between a member variable and a local variable, but I prefer instead to simply avoid such naming clashes. In general I find it too verbose, but that's just me. Other people will have differing opinions.
It is unlikely to ever be a performance concern.
> Bad form/style by 2016 standards?
It isn't the most beautiful, but the intention is absolutely clear. It is generally more 'clean' to have descriptive names, and more common to have a prefix for member variables, but what you've got there is fully functional and obvious both to the human who maintains it and the compiler that consumes it.
> Performance hit?
Not at all.
Many decades ago, back in the 1980s and early 1990s, there were a few early C++ compilers that had minor bugs where they would accidentally dereference the this pointer even though it was already in a register. Unfortunately those bugs were widely discussed, then became a glorified performance problem that people claim today, decades after the bugs were fixed.
It is one of many performance tips and tricks that no longer apply. (Other common mis-applied tricks are using an XOR swap which is a nightmare on modern processors, the Duffs Device that is so bad for performance many optimizers attempt to detect it and replace it, misapplication of the inline keyword that got so bad the best compilers now ignore the keyword, or advice to use ++i instead if i++ for integers used in for loops.)
> Too verbose?
No.
Donald Knuth's advice is still applicable today, 42 years after he wrote it:
Programmers waste enormous amounts of time thinking about, or worrying about, the speed of noncritical parts of their programs, and these attempts at efficiency actually have a strong negative impact when debugging and maintenance are considered. We should forget about small efficiencies, say about 97% of the time: premature optimization is the root of all evil. Yet we should not pass up our opportunities in that critical 3%. A good programmer will not be lulled into complacency by such reasoning, he will be wise to look carefully at the critical code; but only after that code has been identified.
That is exactly what you are doing.
Don't worry about this, or even spend much time thinking about it. It isn't a performance issue.
There are some performance concerns, but they rarely apply. The only time you should really be concerned about performance is after that code has been identified. That means using a profiler to identify the actual parts of your program that are slow, and generally ignoring micro-optimizations everywhere else. Let the optimizing compiler do its job. When a performance problem is identified profile the code, make your changes, then profile again to ensure it is improved.
This all overthinking really, but I'd prefer you rename either the incoming x and y, or the members, and get in the habit of using an initializer list. For this simplistic example, it doesn't really matter though.
The this pointer doesn't bother me in the slightest, I do tend to use it to disambiguate the names as I would often rather not have to distort the incoming parameter names.
In your example however I would actually use an initialiser list. But you could just as easily re-create your scenario with a member function:
Point(int x, int y) : x(x), y(y)
{
}
void Point::set(int x, int y)
{
this->x = x;
this->y = y;
}
Hello everyone,
Not an altogether important question but one I have been wandering about for some time. Say we have the following example.I don't see this very often, and I have heard, and read some snarky remarks with respect to the above code. Just wandering if there are any problems which I don't understand.Point(int x, int y){ this->x=x; this->y=y; }
Bad form/style by 2016 standards?
Performance hit?
Too verbose?
Thanks,
Mike
Point(int x, int y) : x(x), y(y) {
}
In my experience, it's much more common to use a prefix such as m_ instead of this-> to disambiguate member variables.
I personally hate the this-> style for being too verbose... however, accessing a member variable is a performance concern when doing low level programming, and the this-> style makes this fact painfully obvious to the eye and impossible to miss, which could help you spot potential performance problems (e.g. type aliasing causing excessive loads) so that could be a positive for it. If you're not writing the kind of code where you care about whether type aliasing rules are going to cause an excessive load instruction to be generated though, then that positive isn't really applicable to you though.
My personal take is that I _want_ it to be good form and that I find the automatic class scope lookup rules in C++ to be a mistake, but the reality is that those rules do exist and that using this-> unnecessarily is uncommon.
Coding that way won't hurt anything but it will make the code look a little alien or weird to a majority of C++ programmers, and getting in the habit of writing code _for other people_ is a good thing.
Well, you could always use something like Resharper or lint (is that still around?) to enforce that idiom?
Thanks for all the great reply guys.
I just want to make it clear that I don't use the above code snippet, as I rarely see anything of the sort when looking at other people's source code. But once in a while I do come upon it and I was just curious that's all. The initilizer list is my preference.
I also know that many people here work/worked for professional software companies, so I wanted to know some common practices.
Thanks again,
Mike
