the new MIN and MAX macros

2009-05-29
2013-05-28
  • Todor Totev
    Todor Totev
    2009-05-29

    They "leak" to our code base as well as the old ones and we cannot compile unless we undef them. I understand that being headers-only, Win32++ has some unique problems but this is bad.

    Possible fixes:

    1. namespace win32pp { template <class t> t min(t left, t right) { ... } }
    2. #define WIN32PP_MIN(a,b) ...
    and at the end of each and every win32++ header file add
    #ifdef WIN32PP_MIN
    #undef WIN32PP_MIN

    I think even VC 6.0 will compile the first variant.

     
    • Todor Totev
      Todor Totev
      2009-05-29

      Urgh, I was too quick. Obviously you cannot #undef them at the end of every header file, unless you also define them at the beginning as well. Otherwise, the users cannot include two win32++ headers, like "dialog.h" and "listview.h" .

      Really, if VC6 does support templated min/max functions, it'd be best to use them, because they obey the namespace lookup and parameter resolution rules of the language, instead of blindly stomping over whatever code follows them.

       
    • David
      David
      2009-05-30

      The MIN and MAX macros are valid and reasonable. They should not pose a problem in well written code unless of course you have your own MIN and MAX macros defined.  The important thing is to understand why these macros cause an issue with your code.

      You said this ...
      "I understand that being headers-only, Win32++ has some unique problems but this is bad."

      That's rubbish of course, but if you have some issue with using the header-only implementation of Win32++, you might prefer the non-header-only implementation of Win32++ provided with the StaticLibrary sample.

      David

       
      • Todor Totev
        Todor Totev
        2009-05-30

        Here is part of my actual code:

        namespace type_information
        {
        template<>
          struct type_limit<long double,sizeof(long double)>
          {
            static long double MIN() { .... }
        }
        }

        Do you see how they DO pose a problem in well written code?

        The "this is bad" refers to the fact that internal implementation of WIn32++ leaks outside it and stomps over completely unrelated code. I **LIKE** the headers-only implementation of WIn32. It makes it so easy to just put it in a solution and use it.

        My current solution for the problem is to #undef MIN/MAX right after I include win32++ headers so it's just a minor inconvinience.

         
    • David
      David
      2009-05-31

      Yes well there is a slight problem with your code.  The long standing convention for both C and C++ is to name any macro in all capitals, and any non-macro otherwise.  Here you have a C++ function called MIN wich breaks this convention. The correct solution here is to rename your MIN function to something else (possibly Min). You should rename any other all-capital function names while you're at it.

      It's important as programmers that we can spot macros at a glance. Macros behave differently. Not only do they have different scope, but macros pretending to be functions behave differently to real functions under some circumstances. This can lead to subtle bugs.

      No doubt we can both cite examples where macros don't follow this naming convention (min and max for example). That one's actually a mistake on Microsoft (and others) part, and its why we got into this mess in the first place.

      David.

       
      • Todor Totev
        Todor Totev
        2009-05-31

        It looks like #undef'ing MIN and MAX after including all win32++ headers is the most viable solution.