Jump to content
  • Advertisement
Sign in to follow this  
remdul

c++ LoadImageA won't #undef [solved]

This topic is 4079 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

For some weird reason LoadImageA won't #undef for me. I do this after all #includes, right before the function that makes a CImage::LoadImage() call.
#include <windows.h> // winuser.h defines LoadImage as LoadImageA
#include "image.h" // my image class (CImage)

#ifdef LoadImageA
#undef LoadImageA
#endif

void FooBar()
{
 LoadImage("blah.png");
}
And yet the compiler still complains:
error C2039: 'LoadImageA' : is not a member of 'CImage'
I'm confused. I'm #undeffing lots of garbage that is defined in the windows/directx headers (PlaySound etc.), but this one just does not work. [Edited by - remdul on April 20, 2007 9:26:32 AM]

Share this post


Link to post
Share on other sites
Advertisement
Quote:

I'm confused. I'm #undeffing lots of garbage that is defined in the windows/directx headers (PlaySound etc.), but this one just does not work.


There are a number of reasons this is a bad idea. First and foremost, it is very difficult to maintain. It's also hard to do correctly in the rare cases you do need to do it; the fact that you seem to need to do it a lot suggests you might be doing it wrong.

The first thing, in my mind, to consider is whether or not you even need to -- that is, perhaps you don't need the name "LoadImage." Your class is called CImage; to me, CImage::Load is perfectly informative on its own (I would even drop the 'C' prefix, as I find it to be a useless naming wart). Perhaps LoadFromFile() or LoadFromMemory() if you have multiple means of loading. That fixes this initial issue of poor naming on your part.

The next thing is to consider whether or not it matters. If your class definition is such that you require windows.h to be included in the header, the macro will safely replace both the name in the class definition and the name in the implementation, and overloading will do its sexy thing, and you'll never notice the difference unless you dump the binary; the cases where this matters (such as interop with other languages) are few, so this usually takes care of most additional issues.

The third thing to consider is whether or not you even NEED windows.h included. Ideally you should include such a massive header as infrequently as possible. If you're #undef'ing all the Windows compatibility macros, are you even using anything from the API that requires the header to be included?

The final thing to consider, if you must avoid the macro renaming your method, is whether or not #undef is the proper way to remove the function. There are many preprocessor symbols the Windows headers obey, such as NOMINMAX (I think) to disable the "min" and "max" macros. There might be one (LEAN_AND_MEAN, VC_EXTRALEAN, et cetera) that removes the macro for the functions your interested in.

Finally, if you really must insist on doing it the clunky #undef way, the problem probably lies elsewhere -- the small snippet you've post should be okay; you'll need to post more.

Share this post


Link to post
Share on other sites
Beyond what jpetrie said (which is good advice - pay attention) there's one possible way you could be going wrong in the code you posted. If image.h defines the CImage class (which is logical to assume) and CImage is supposed to have a LoadImage member (logical from the error message you posted), then it looks like the LoadImageA macro is coming into effect inside of image.h.

Remember, when you #include a file, it is basically copy and pasted into the parent file. That means that the contents of image.h will appear above your #undef, and in this case the #undef will only come into effect after image.h is processed. You should either put the #undef into image.h or above it in the .cpp file; or, better still, just follow jpetrie's recommendations.

Share this post


Link to post
Share on other sites
Although I would probably follow jpetrie's advice and look into alternatives, shouldn't you be using #undef LoadImage rather than #undef LoadImageA?

Share this post


Link to post
Share on other sites
Quote:
Original post by jpetrie
Quote:

I'm confused. I'm #undeffing lots of garbage that is defined in the windows/directx headers (PlaySound etc.), but this one just does not work.


There are a number of reasons this is a bad idea. First and foremost, it is very difficult to maintain. It's also hard to do correctly in the rare cases you do need to do it; the fact that you seem to need to do it a lot suggests you might be doing it wrong.

There is also the fact that when you do such modification in a header file, how can we know what the exact reason of the failure is? You seem to believe that "lots of garbage" in these headers - that's not true. The Windows SDK header files are written by very competent people, they are heavily tested by hundreds of thousands people arount the world, and every single line of this file is important, even if you don't fully understand this line.

So I suggest you to restore the original file, and to cope with it.

Now, to answer your question, LoadImageA is not a preprocessor symbol - it's a function name - thus you simply can't undef it. Only preprocessor symbols can be undefined.

Best regards,

Share this post


Link to post
Share on other sites
Yes, this is one of those cases where I have little other options.

1) Yes, including windows header files should be avoided at all costs, but I can't avoid that in this case since I actually need it.

2) LoadImage is the best name for what I'm doing. I actually have more functions in this class, LoadBMP, LoadPNG, LoadJPG etc. and LoadImage is a wrapper that checks the extension/header of a file then forwards it to any of those functions.

3) Changing a name of a function because some other header file is making a mess of things is no valid reason IMHO. :)

4) I'm not undeffing that much really, just a few unavoidable cases.

5) Trying the LEAN_AND_MEAN trick, I actually used that elsewhere (why I didn't think of that?). Thanks. :)

Share this post


Link to post
Share on other sites
Ow, forget it. I messed up. It was just a typo, I was undeffing the wrong thing.

Thanks for the advice/hints tho, and for wasting your time. ;)

Share this post


Link to post
Share on other sites
Quote:
Original post by remdul
Yes, this is one of those cases where I have little other options.

Of course you have. It's not like if someone else is threatening your life [smile]

Quote:
Original post by remdul
1) Yes, including windows header files should be avoided at all costs, but I can't avoid that in this case since I actually need it.

Including it is not a problem. Modifying it is.

Quote:
Original post by remdul
2) LoadImage is the best name for what I'm doing. I actually have more functions in this class, LoadBMP, LoadPNG, LoadJPG etc. and LoadImage is a wrapper that checks the extension/header of a file then forwards it to any of those functions.

Your function lies in a Image class, so you can be pretty sure that calling it Load (and not LoadImage) will not make people think that it's loading watermellon on the back of a truck [smile].

Quote:
Original post by remdul
3) Changing a name of a function because some other header file is making a mess of things is no valid reason IMHO. :)

That deserves a "uh... !?" Semantically speaking, you are true - this is not a valid reason. But technically speaking, what other choice (I mean, a choice that would make sense) do you have? Changing the header file is, I restate it, not an option at all. There is just too many impacts here, and some of them are not visible in your projects. But what if you include another (more or less "standard") header file that relies on the part you modified? Are you going to modify it too?

So go ahead and modify your function name. That's the best thing to do in that case.

Quote:
Original post by remdul
4) I'm not undeffing that much really, just a few unavoidable cases.

One character is enough to create a Royal Mess. The best thing to do here would be to #undef right after the file is included, not inside the file itself. I assumed in my previous answer (and in this one) that you modified the windows.h header. I stress it again (yeah, I know, I stress it too much [smile]): this is not the right thing to do.

Best regards,

Share this post


Link to post
Share on other sites
Quote:
Of course you have. It's not like if someone else is threatening your life

You wouldn't believe me if I told ya. ;)

Quote:
Including it is not a problem. Modifying it is.

I'm not modifying the windows header files at all. I'm stupid, but not THAT stupid!

Quote:
Your function lies in a Image class, so you can be pretty sure that calling it Load (and not LoadImage) will not make people think that it's loading watermellon on the back of a truck.

I know what you mean. However, the class also does a LOTS of other stuff. It can also load texture cache files, among other things. So you have LoadPNG, LoadPalette, LoadCache etc. Naturally, I named it LoadImage for consistency. Often consistency is far more important than that other thing, especially in large projects.

Quote:
One character is enough to create a Royal Mess. The best thing to do here would be to #undef right after the file is included

Yes, that was exactly what I was doing. But I was undeffing LoadImageA instead of LoadImage, oops!

Share this post


Link to post
Share on other sites
Quote:

I know what you mean. However, the class also does a LOTS of other stuff.

Then it violates the single-responsibility principal and is poorly designed. It shouldn't need to do "LOTS" of stuff. That's brittle. Prefer many classes that each do one thing to one class that does many things. Read this.

Quote:

It can also load texture cache files, among other things. So you have LoadPNG, LoadPalette, LoadCache etc. Naturally, I named it LoadImage for consistency. Often consistency is far more important than that other thing, especially in large projects.

Not neccessarily, and I fail to see how adding Image to Load is any more consistent (all your other functions are LoadTYPE, Image is not a type, and Image is also redundant since it's part of the class name).

In short, your design is flawed, and this is still well within your ability to fix without having to resort to nasty hacks.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

Participate in the game development conversation and more when you create an account on GameDev.net!

Sign me up!