Jump to content
  • Advertisement
Sign in to follow this  
Mithrandir

Magic Booleans

This topic is 4910 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

I came across an interesting problem today. I've entirely phased out "magic numbers" in my programs; haven't had one of those in at least 3-4 years (except for 0, of course). The reasoning, of course, is to avoid unreadable code:
if( x == 86.4 )  // what the F does 86.4 represent?
{
    // blah
}


Ok. So onto something else that bothered me. Magic Booleans. I had a function:
    public class EntityNotifier
    {
        public static void PreNotify( long entity, Notification notification, bool recursive )
        {
            Entity e = Database.Get( entity );
            e.PreNotify.Notify( notification );

            if( recursive )
            {
                foreach( long child in e.Children )
                {
                    PreNotify( child, notification, recursive );
                }
            }
        }
    }


(this is rough prototype code, by the way, don't nitpick it) Okay, so that's all cool and all, but what about the callers of the code? They'll do something like this:
EventNotifier.PreNotify( roomid, leaveroom, true );


The code is pretty readable... except for that boolean sticking at the end. I oftentimes find myself, whenever I see a boolean, wondering "what the F does that boolean mean?". So, I was thinking... would it not be more appropriate to use enums instead of booleans to make code more clean and robust?
public enum RecursionState
{
    None,
    Recursive
}

<snip>

EventNotifier.PreNotify( roomid, leaveroom, RecursionState.Recursive );


To me, this solution makes the code much more legible, and I think I'll prefer doing this from now on... but I was wondering what other people think? Is this too much extra work, or do you guys generally think the end result is worth it?

Share this post


Link to post
Share on other sites
Advertisement
I should note that I've already had one benefit of using enums rather than bools; I had a class that accepted true/false in the constructor to tell the class that it was in one of two mutually exclusive states... and I switched it over to an enum to represent the states.

Later on, I realised that the class required 3 mutually exclusive states, unrepresentable by a bool, and luckily for me, the enum was already there, so adding the new state was trivial.



Plus, enums in C# are heaven compared to that pathetic clusterfuck of a trainwreck they call enums in C++ (grin), so I don't have to worry about playing messed up namespace games to avoid conflicting enum names (gee, there are about 100 different enums that use the state "None". Thanks for making this situation almost uncodable in C++!!)

Share this post


Link to post
Share on other sites
I use something like that all the time for error reporting, instead of just returning bool from all functions to represent failure/success.


class CThing
{
public:

enum TypeError
{
Error_NoError = 0,
Error_NoMemory,
Error_NoFile,
Error_BadPhaseOfMoon,
Error_MonthEndingInR,
Num_ErrorTypes
};

static const char *GetErrorText(const TypeError eType)
{
switch (eType)
{
case Error_NoMemory:
{
return "Not enough memory";
}
break;

<snip>
}
}

TypeError FunctionCall()
{
//return Error_NoError on success, whatever else on failure
<snip>
}
};


int main()
{
CThing inst;
CThing::TypeError eError = inst.FunctionCall();
if (eError != CThing::TypeError::ErrorNoError)
{
cout << "Hey dumbass: " << CThing::GetErrorText(eError);
return 0;
}

<snip>

}





Share this post


Link to post
Share on other sites
I think it's an improvement, but it doesn't address the problem where you need to do something like pass two strings to a function - example would be MessageBox() that takes both a message and a caption for the box. Which is the caption and which is the message? It may not be obvious at a glance.

One of the features I particularly liked in Objective C (something that I believe it pinched from Smalltalk) is function calls that explicitly name parameters; it makes code clearer and also gives you more flexibility with things like default arguments. Your call to PreNotify would look something like this:

PreNotify(entity: roomid, notification: leaveroom, recursive: true)

You can now see immediately what each parameter means, eliminating the need for extra enum types like you describe. It would also be trivial to allow switching the order of arguments or leaving 'notification' at a default value.

This is actually related to one of the things that the Direct3D team have changed for Direct3D 10 - a great number of functions have been changed to take pointers to structures instead of lists of arguments. This means that you're able to write out explicit "args.parameter = value;" code before making the call.

Share this post


Link to post
Share on other sites
Yeah, that does make a lot of sense. It's kinda like when I was working with python and calling functions, I would go the extra distance and do this.

SomeFunction(xval = 21, yval = 44)

instead of

SomeFunction(21, 44)


It makes it a ton easier to learn to work with that language, since it isn't type strong. Yeah, enumerating instead of relying on raw booleans is a good idea. In C++ though, You do it with #define, not enum. ;)

Share this post


Link to post
Share on other sites
Booleans are sort of an extreme situation (because they are only two-valued) of a more general problem, which is positional argument lists. In the function call cycleWidgetDiscriminator(context, NULL, 1, true), I'd argue that all of the last three parameters are confusing.

Now, you could argue that 1 is a magic number, and do something like this:

const int WIDGETMULTIPLIER_ONE = 1;
...
cycleWidgetDiscriminator(context, NULL, WIDGETMULTIPLIER_ONE, true)


...but hell, why not go to town?


const int WIDGETMULTIPLIER_IDENTITY = 1;
const WidgetOptions* WIDGETOPTIONS_NONE = NULL;
const bool DOADJUSTCHILDREN = true;
...
cycleWidgetDiscriminator(context, WIDGETOPTIONS_NONE, WIDGETMULTIPLIER_ONE, DOADJUSTCHILDREN)



... ugh. I might care what the identity element is elsewhere in the program, I might not. I certainly don't like the idea of a global DOADJUSTCHILDREN, and I don't like setting up an ad-hoc CYCLEWIDGETDISCRIMINATOR_ARGUMENTS_DOADJUSTCHILDREN namespace either. I'm a grownup, and I can remember that "adjustChildren=true" means do adjust the children, and "adjustChildren=false" means don't. Let's make that a little better scoped.

{
WidgetOptions* widgetOptions = NULL;
int multiplier = 1;
bool adjustChildren = true;
cycleWidgetDiscriminator(context, widgetOptions, multiplier, adjustChildren)
}

Now this, of course, is a redirection. We aren't mentioning anymore that "1" is the identity element for multiplier; instead, we're more interested in mentioning that the third argument is a multiplier. I think this is a good thing: When you hear "multiplier", you know from high school algebra that 1 is the identity element. And as a programmer, you already know what true and false are, and you probably realize that NULL in a pointer context means "don't use". Wrapping it all up in a scope makes it clear that these variables only exist as arguments.

Of course, what would be even better:

cycleWidgetDiscriminator(context=context, widgetOptions=NULL, multiplier=1, adjustChildren=true)

Cannot do, of course, at least in C++. But it seems to me that that's really what you want from the language. Some IDEs will do it for you while you're typing, but that's not much comfort when reading code (especially printed-out code).

EDIT: Beaten like a red-headed stepchild. Which, in fact, I currently am. Whoops.

Share this post


Link to post
Share on other sites
BTW: I should clarify that I do NOT agree with the idea of enumifying things that should be booleans. That's a hack which only works on discrete values, and its form (datatype) is not suited to its function (argument). Would you make an enum for every non-argument local boolean in your program? No! You'd name them correctly. Similarly, this should be a call for named arguments, not enumed bools.

Share this post


Link to post
Share on other sites
Quote:
Original post by Sneftel
BTW: I should clarify that I do NOT agree with the idea of enumifying things that should be booleans. That's a hack which only works on discrete values, and its form (datatype) is not suited to its function (argument). Would you make an enum for every non-argument local boolean in your program? No! You'd name them correctly. Similarly, this should be a call for named arguments, not enumed bools.


Interesting.

I never thought about it this way, and it seems to make sense.


For example, in the function itself, the parameter is named "recursive", and it makes perfect sense within the context of the function.

But it doesn't on the outside.

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.

GameDev.net is your game development community. Create an account for your GameDev Portfolio and participate in the largest developer community in the games industry.

Sign me up!