0

Please see the small code below:

void TestPluginAPI::MouseMove(int nX, int nY)
{
    INPUT input;
    DOUBLE fScreenWidth = GetSystemMetrics( SM_CXSCREEN )-1; 
    DOUBLE fScreenHeight  = GetSystemMetrics( SM_CYSCREEN )-1; 
    int plusY;
    if (FullScreenCheck() == 1) {int plusY = getmidY() + 8.0f;}
    else {int plusY = getmidY();}
    int plusX = getmidX();
    DOUBLE fX = plusX*(65535.0f/fScreenWidth) + (nX*(65535.0f/fScreenWidth));
    DOUBLE fY = plusY*(65535.0f/fScreenHeight) + (nY*(65535.0f/fScreenHeight));

    RtlZeroMemory(&input, sizeof(input));
    input.type = INPUT_MOUSE;
    input.mi.dwFlags = MOUSEEVENTF_MOVE|MOUSEEVENTF_ABSOLUTE;
    input.mi.dx = (LONG)fX;
    input.mi.dy = (LONG)fY;

    SendInput(1, &input, sizeof(input));
}

The thing is that the compiler gives me a warning, that plusY is undefined, and obviously if I try to run this function in a compiled plugin (it's from plugin code, debugging it thru javascript console in a browser) it crashes because plusY is undefined.

It seems that if I can't define variables in function through if-else, then how can I do this?

Eric Skroch
  • 27,381
  • 3
  • 29
  • 36
Max Yari
  • 3,617
  • 5
  • 32
  • 56

5 Answers5

4
{int plusY = getmidY() + 8.0f;}

int plusY stops existing at the } in both if statements

int plusY inside brackets {} is a different variable than int plusY outside.

You should do this

int plusY;
if (FullScreenCheck() == 1) {plusY = getmidY() + 8.0f;}
else {plusY = getmidY();}
  • 1) Even without the braces, the variable in a then or an else branch of an if ceases to exist at the end of the branch, and 2) using the ternary operator in the initialization clause, rather than an `if` after, makes the code far more readable. – James Kanze Apr 01 '13 at 00:23
2

Variables defined in one of the branches of an if have the scope of that branch only. Even if you eliminated the braces (which would be legal here):

if ( fullScreenCheck() == 1 )
    int plusY = getmidY() + 8.0f;
else
    int plusY = getmidY();

will result in a plusY whose scope ends after the if.

The best way of handling this is:

int plusY = fullScreenCheck() == 1
            ? getmidY() + 8.0f
            : getmidY();

Because of the restrictions on what you can do in a single expression, it's not always possible to do it, but whenever you can, it results in far more readable code.

James Kanze
  • 150,581
  • 18
  • 184
  • 329
1

plusY is destructed from memory at the closing brace of the if statement. If you need to use its after the if statement then you should declare it outside the block:

int plusY;

if (...)
{
    plusY = getmidY() + 8.0f;
}
David G
  • 94,763
  • 41
  • 167
  • 253
1
int plusY;

You have declared a variable named plusY which is available to your entire function

if (FullScreenCheck() == 1) {int plusY = getmidY() + 8.0f;}

This declares another variable which also happens to be named plusY, but this new variable is only available inside of the if statement.

else {int plusY = getmidY();}

And now you declare a third variable with the same name which is only available in the else clause.

To fix this and use the same variable in all locations, change your code to

int plusY;
if (FullScreenCheck() == 1) {plusY = getmidY() + 8.0f;}
else {plusY = getmidY();}

p.s. As an aside, you should learn about typical code formatting practices and conventions.

p.p.s. You should also initialize plusY to a value which is otherwise invalid so you can easily track down any problems. Alternatively, you can use the ternary operator in order to avoid this issue entirely. In this situation, the ternary operator is most likely the best solution.

Code-Apprentice
  • 81,660
  • 23
  • 145
  • 268
  • And leave `plusY` uninitialized after its definition? It would be far better to use the ternary operator here. – James Kanze Apr 01 '13 at 00:21
  • @JamesKanze Does it matter that much when the if...else immediately follows the declaration? Also, simply suggesting the ternary operator doesn't explain *why* the error occurs in the first place. – Code-Apprentice Apr 01 '13 at 00:23
  • From a language point of view, it only matters if you use the variable before assigning to it. From a readability point of view, it means that whoever is reading the code has to go looking for the initialization, and verify that it occurs in all of the branches. Whereas if he sees `int plusY = ...`, it's clear from the start---I have an initialized variable. – James Kanze Apr 01 '13 at 00:25
  • @JamesKanze IMO, suggesting that the OP use the ternary operator confuses the issue. (And most likely can confuse the OP at their likely level of competence with C++.) – Code-Apprentice Apr 01 '13 at 00:26
  • @JamesKanze Most likely this is a personal project and the only reader will be the OP (and SO users when they ask questions). – Code-Apprentice Apr 01 '13 at 00:27
  • @JamesKanze Because the current error is about scope of variable declarations. By using the ternary operator, you remove the scope of the if statement entirely. Obviously the OP is a beginner and needs a reminder about the scope issues involved in the original code. This same issue can arise in many other situations where the ternary operator isn't a viable solution. – Code-Apprentice Apr 01 '13 at 16:06
  • OK. I misunderstood your point. I agree that any correct answer should point out the scope issues---as you say, there are cases where there isn't any other solution but an uninitialized variable followed by an `if` or a `switch`. But I also think it useful to remind him of the ternary operator, since in cases where it is possible (like his example), it leads to simpler and more understandable code. – James Kanze Apr 01 '13 at 19:57
  • @JamesKanze Agreed as far as the ternary operator goes. And when you must declare a variable without knowing its value, first initialize it to some value which otherwise never occurs, if at all possible. – Code-Apprentice Apr 01 '13 at 22:33
1

Your entire if/else can be replaced (with scoping rules fixed) with this one line.

int plusY = (FullScreenCheck() == 1) ? getmidY() + 8.0f : getmidY();
Drew Dormann
  • 59,987
  • 13
  • 123
  • 180