if statement problem

[Tanatos]

Hello, I've designed a macro for dynamically casting pointers.
My macro looks like this:

#define using(interface, object) if(object) if(interface * self = dynamic_cast<interface *>(object))

and can be used like this:

using(Interface, object_ptr)
{
    // do something using the "self" pointer which is of type Interface *
}

else    //ambiguous else
    // some more code 

Don't mind the fact that I'm using the keyword using, the compiler doesn't complain.
The first if checks whether there's a valid object behind the object pointer. The second one does the dynamic_casting and checks whether the cast was successful or not. If it is not, it skips the block of code that follows.
My problem is that I have two if-statements and using else after the macro would be ambiguous (even the compiler warns me so)
I decided to combine the two if-statements into one like this:
if(object && interface * self = dynamic_cast<interface *>(object))
but the compiler spits out and error message saying

expected primary-expression before '*' token

So my conclusion is if you declare something inside an if-statement it must be the first thing in the statement. Is there a workaround or some other way I can use only one if-statement with two checks and a declaration after the first check?

[hamsterman]

What do you think dynamic_cast would return if you passed 0 to it ? The first condition is redundant.

[andywestken]

I'm not sure the code your macro evalates to would even compile.

???

if(object_ptr)
{
    if(Interface * self = dynamic_cast<Interface *>(object_ptr))
    {
        // FAIL to compile as there is no variable called self in scope!?!
    } 
}

I've never see a variable declared within an if(..)

if(object_ptr)
{
    Interface * self = NULL;

    if(self = dynamic_cast<Interface *>(object_ptr))
    {
        // Use self
    } 
}

should work, but (a) it's not what you macro evaluates to, and (b) poor style to assign within an if condition.

Andy

[Tanatos]

@hamsterman
if you dynamic_cast a NULL pointer or a pointer to a memory that was freed (deleted) you get a segmentation fault, that's why I check the pointer with the first if-statement.

@Andy

poor style to assign within an if condition.

is it? Because I think it's quite useful when you want a temporary variable just for the scope of the if-statement.
The macro is 100% perfectly working (I'm using the GNU GCC compiler version 4.5.2) I'm not sure if that matters.
I don't usually declare variables within an if statement but since it is possible and this is a macro I thought it would be convenient enough to do so. There's no problem in that

if( int A = f() ) // A gets created, the return value of f() is assigned to A, the if-statement checks whether the new value of A != 0
{
    // A is still in scope so you can do things like
    cout << A << endl; // no problem with this code
}  // This is where A goes out of scope 

The code you provided would be perfect for my case but it's not actually the one my macro evaluates to. There's a slight difference (which brings the problem)
My macro evaluates to:

if(object_ptr) // if the pointer isn't NULL
    if(Interface * self = dynamic_cast<Interface *>(object_ptr))
    // if the dynamic_cast didn't return NULL (this means everything is OK with the cast)
    {
        // this is the code block following the macro 
    }

There's no way I can close the brackets of the first if-statement because the last curly bracket (in your code) in no way can be part of the same macro. I'll have to close it manually which is something I'm trying to avoid.

I'm trying to combine the two if-statements into one so when I use else right after the macro it would correctly lead to the code where the whole macro did not execute.

Consider the following code

using(Interface, obj_ptr)
{
   // some code if the cast was successful
}

else
{
    cout << "There was a problem with the pointer" << endl;
}

The control wouldn't reach the cout if the pointer equals NULL because the else refers to the second if and not the first one. I can make the else to refer to the first if by either enclosing the inner if in curly brackets or appending else; at the end of the block following the macro. But both of those solutions involve writing additional code outside the macro just to make it work.

[Disch]

4 things:

1) to avoid the lingering extra-if you have in there, I would use the ternary operator.

#define USING(Interface,obj_ptr)  \
    if(Interface* self = (obj_ptr) ? dynamic_cast<Interface*>(obj_ptr) : 0) 

2) Using the using keyword is a terrible idea. Sure it works for your macro, but if you try to use using for it's intended use, it will barf all over you. Macros typically use all caps (ie: "USING") to avoid that issue.

#define using ...

using namespace std;  // Compiler barf 

That's besides the fact that this will confuse the hell out of anyone that looks at your code (including a future version of yourself)

3) Instead of using 'self' as an assuming variable name, consider passing the variable name to the macro:

#define USING(Interface,self,obj_ptr) //... 

4) If you're downcasting often enough to need a macro for it, your design is probably flawed in the first place. Downcasting is very rarely needed.

[Tanatos]

1) that was the answer I was expecting. That should work. Forgot about this kind of conditional expression.
2) I don't have a problem compiling a code using this macro and the ordinary using keyword. Anyway, I can change it whenever I want to. It's just a temporary name.
3) I need only a temporary pointer so after I leave the cast scope I would be left with only the original object pointer (you know, something like the this keyword)
4) Well if my design is flawed sooner or later I'll know. As for now I don't have any reason not to use downcasting. It may be rarely used but I need it and it's doing a perfect job for now.

Anyway thanks for the suggestions, I'll take them into consideration and thanks for the solution to my problem.

[Disch]

I don't have a problem compiling a code using this macro and the ordinary using keyword

I'm dubious. It's a clear conflict.

Anyway, I can change it whenever I want to. It's just a temporary name.

The longer you wait, the harder it gets to change. If you change it now, you might only have to update a small handful of areas in your program. If you wait, you might have to change dozens or hundreds depending on how often it's used.

Honestly, I'm really surpised you are even hesitating to change it. I mean using is a reserved keyword by the language. It's clearly a terrible choice. You never even should have considered going with it in the first place.

[hamsterman]

Tanatos wrote:

if you dynamic_cast a NULL pointer <...> you get a segmentation fault

Is that so?.. That's weird.

_{C++ standard section 5.2.7 part 4 wrote:}

If the value of v is a null pointer value in the pointer case, the result is the null pointer value of type R.

And it seems to work fine for my VC++. What compiler are you using?

Disch wrote:

If you're downcasting often enough to need a macro for it, your design is probably flawed in the first place.

I have doubts about that. Not that I really know something about good design..
Downcasting offers a type of polymorphism which allows you to have all the relevant code in one place. This is very comfortable. Scala seems to recognise this as a perfectly valid design with its match{ case ... } construct..
What are your arguments about this?

[Tanatos]

Actually you're right, hamsterman, dynamic_casting a NULL pointer returns a NULL pointer, it doesn't raise a segmentation fault signal. I don't know why I got confused. At some point I received a SIGSEGV so I assumed I was casting a NULL pointer but now I can see this is not the case. It was just a pointer to unallocated memory. Which means the first if in my macro is useless. At least I now know how to combine multiple conditional expressions into one if there's a declaration in one of them. And that was the main purpose of the topic.

@Disch
I can see why it is a bad idea. It just sounds so natural and self explanatory: using this interface of that object. But you've got a point that it would look confusing to others. That's why I changed it to USING as you suggested.