snapValue prototype


#1

the current tip has the following prototype for Slider::snapValue

virtual double snapValue (double attemptedValue, bool userIsDragging);

while the .cpp has the following:

double Slider::snapValue (double attemptedValue, const bool)

Any reason for which the “const” attribute for the bool parameter has been removed?

In either case, one of the two prototypes should be changed for consistency with the other, I think.


#2

I’ve been removing a few unnecessary consts in the definitions lately. For a primitive type, the compiler ignores the const in the declaration, but it makes it a bit more cluttered and hard to read, so I’ve removed them. Having one in the definition, however, like any other const, won’t do any harm, and could catch the occasional slip-up, so is a good thing. The c++ standard specifically lets you make them different.


#3

yes, but it generates some warnings on Visual Studio 2008


#4

Ah, good old visual studio. You could disable the warning, or remove the const in your overloaded function definition. Either way it’ll make no difference to the code.


#5

It also makes a big difference when you grep the code (you won’t find the definition for a declaration).
Also maintenance is easier when you have exact same signature (that is, when you add a “&” to a parameter in the declaration, then the “const” in cpp make a big difference as the signature will change).

I think it’s sane practice to use the same definition than the declaration in all case.


#6

(There’s no point grepping for that string anyway - the definition and declaration may use different variable names, or have different spacing…)

The way I’m doing it now is the style encouraged in Sutter/Alexandrescu’s C++ Coding Standards. It’s not particularly important, and isn’t something I’d ever thought about until I read their book, but I definitely agree that this is a clearer way of doing it.


#7

This is wrong. Look at this code shows:

struct A
{
   virtual void method(const int a) = 0;
   virtual ~A() {}
};
struct B : public A
{
   void method(int b) {}
};
int main(int a, char ** v)
{
    B test; // error C2259: 'B' : cannot instantiate abstract class due to following members:
                // 'void __thiscall A::method(const int)' : pure virtual function was not defined
    return 0;
}

Well, if you understand grep as VS’s ‘find in files’, sure. grep does regular expression, and it’s definitively useful when you have tons of overload (like in << operator), and “find in file” would be useless (unless you’re addict to Find next button), while grep for “operator\s+<<\s*(\s*const\s+int” instantly goes to “operator << for const int”.

My second point still hold, if you use the same code for both, it’s still easier for noobies to “find in files” with the method signature minus return type (of course).
I usually do copy & paste of the signature when writing the cpp file implementation.

Anyway, this has no impact to the code quality and the way it’s working, no impact to the client code functionality, but it’s a pain that we have to fix all our listeners in our code.
I hope you’ll release a 1.51 version soon and try to freeze the “API” because we always lag and spend too much time maintaining such “little” changes with so great impact.


#8

In fact, in virtual methods I’ve never deliberately used const parameters, but in scanning through the code recently I spotted a few places where I’d accidentally done so. Seems like a good time to correct that - sorry if it means there are a few places where a crappy piece of junk like VC6 will give an error (I assume you’re talking about VC6, because all other compilers would have no problem with your example code).

[quote]
I hope you’ll release a 1.51 version soon and try to freeze the “API” because we always lag and spend too much time maintaining such “little” changes with so great impact.[/quote]

That’s exactly what I’m doing - I’m not adding any new features at the moment, just trying to finish any minor tinkering like this, and get it all stable.


#9

You guessed right, I’ve tried in that “crappy piece of junk like” VS6. I’m amazed that other compiler should accept such different overload, maybe it’s time for me to read the C++ standard again.
I promise all new project will be using a decent compiler (if I could throw all Microsoft in the same time…) but for now, I maintain a big piece of work using VS6 and I can’t change.

Great, great, great!!! I really hope you’ll release soon.

BTW, I’ve had to modify “const”-ness in my last pull, and initially thought it was again a Git’s commit omission, but now I understand why you haven’t see such compiler error.
I’ll pull again by end of week and report any limbo left.


#10

FYI: VC8 does compile everything correctly, but gives a warning that earlier versions of the compiler didn’t behave correctly.


#11

We don’t have VS8 in our company (we have VS2005 and VS6, and of course gcc for posix stuff). Is there any real advantage to VS2008 (except increased memory & disk usage and the usual “newer is better”) ?
Does it mean that the current code will fail in VS2005 ?
Do you have the exact warning text ?

Also, the code compiles with no warning or error in Comeau. I’ll really need to read the standard…


#12

I’ve no idea what VC2005 would do, I don’t use it myself. The warning just says that in older versions, the const is treated differently.

It’s actually standards-compliant. And free.


#13

Maybe more standard compliant, but it’s definitively not free (unless you are speaking about the “Express” edition, but since there is no support for any extensions, to me, it’s only a slow interpreted frontend for a compiler you’ve to download and setup yourself). I hope you’ll create something very cool with the Jucer Reloaded, so that we don’t need to fire up a text editor that consume 500MB just to have colored lines with a “+” symbol on the left…