Style question using enums

[Paul N]

I'm attempting to write a Bridge program, and it's getting quite messy.

At first I simply used a number for each bid, so my code had a lot of "magic numbers" in it. I got rid of some by using an enum, as follows:

enum { B_PASS = 0, B_DOUBLE = 1, B_REDOUBLE = 2,
B_1C = 15, B_1D = 18, B_1H = 21, B_1S = 24, B_1NT = 27,
B_2C = 30, B_2D = 33, B_2H = 36, B_2S = 39, B_2NT = 42,
B_3C = 45, B_3D = 48, B_3H = 51, B_3S = 54, B_3NT = 57,
B_4C = 60, B_4D = 63, B_4H = 66, B_4S = 69, B_4NT = 72 };

This lets me do code such as

if (WentMP(B_1NT, B_2NT)) { // rebid after 1NT - 2NT
if (pc == 14 || (pc == 13 && tens >= 2)) { mess = _TEXT("Raise"); bid = B_3NT; }
else { mess = _TEXT("Not got 25 points"); } }

which is relatively clear.

However, now that I'm trying to deal with suit bids as well it seems silly to deal with each bid separately. I have an enum

enum { clubs, diamonds, hearts, spades, notrumps };

and a function

int b(int level, int suit) { return 15 * level + 3 * suit; }

which allow me to use for instance either B_1NT or b(1, notrumps) as the value for a one notrumps bid.

Part of me thinks I ought to update the first enum to be more like

enum { B_1NT = b(1, notrumps) }

but this does not work, even if I make b inline.

Presumably if would work if I made b a macro, but these are generally discouraged. I'm guessing that some syntax using constexpr might work (I've never used one of them before) but would I need to include the code for b twice, once for the constants and once as a normal function?

All thoughts welcome!

[Paavo Helde]

17.01.2023 14:29 Paul N kirjutas:
> I'm attempting to write a Bridge program, and it's getting quite messy.
>
> At first I simply used a number for each bid, so my code had a lot of "magic numbers" in it. I got rid of some by using an enum, as follows:
>
> enum { B_PASS = 0, B_DOUBLE = 1, B_REDOUBLE = 2,
> B_1C = 15, B_1D = 18, B_1H = 21, B_1S = 24, B_1NT = 27,
> B_2C = 30, B_2D = 33, B_2H = 36, B_2S = 39, B_2NT = 42,
> B_3C = 45, B_3D = 48, B_3H = 51, B_3S = 54, B_3NT = 57,
> B_4C = 60, B_4D = 63, B_4H = 66, B_4S = 69, B_4NT = 72 };
>
> This lets me do code such as
>
> if (WentMP(B_1NT, B_2NT)) { // rebid after 1NT - 2NT
> if (pc == 14 || (pc == 13 && tens >= 2)) { mess = _TEXT("Raise"); bid = B_3NT; }
> else { mess = _TEXT("Not got 25 points"); } }
>
> which is relatively clear.
>
> However, now that I'm trying to deal with suit bids as well it seems silly to deal with each bid separately. I have an enum
>
> enum { clubs, diamonds, hearts, spades, notrumps };
>
> and a function
>
> int b(int level, int suit) { return 15 * level + 3 * suit; }
>
> which allow me to use for instance either B_1NT or b(1, notrumps) as the value for a one notrumps bid.
>
> Part of me thinks I ought to update the first enum to be more like
>
> enum { B_1NT = b(1, notrumps) }
>
> but this does not work, even if I make b inline.

Use constexpr or consteval, depending on if you want to use b() also
with non-compile-time arguments or not.

constexpr int b(int level, int suit) { return 15 * level + 3 * suit; }

[Öö Tiib]

On Tuesday, 17 January 2023 at 14:29:51 UTC+2, Paul N wrote:
> I'm attempting to write a Bridge program, and it's getting quite messy.
>
> At first I simply used a number for each bid, so my code had a lot of "magic numbers" in it. I got rid of some by using an enum, as follows:
>
> enum { B_PASS = 0, B_DOUBLE = 1, B_REDOUBLE = 2,
> B_1C = 15, B_1D = 18, B_1H = 21, B_1S = 24, B_1NT = 27,
> B_2C = 30, B_2D = 33, B_2H = 36, B_2S = 39, B_2NT = 42,
> B_3C = 45, B_3D = 48, B_3H = 51, B_3S = 54, B_3NT = 57,
> B_4C = 60, B_4D = 63, B_4H = 66, B_4S = 69, B_4NT = 72 };
>

Nameless type, SCREAMING CAPS names and then using just ints anyway?

> This lets me do code such as
>
> if (WentMP(B_1NT, B_2NT)) { // rebid after 1NT - 2NT
> if (pc == 14 || (pc == 13 && tens >= 2)) { mess = _TEXT("Raise"); bid = B_3NT; }
> else { mess = _TEXT("Not got 25 points"); } }
>
> which is relatively clear.
>
> However, now that I'm trying to deal with suit bids as well it seems silly to deal with each bid separately. I have an enum
>
> enum { clubs, diamonds, hearts, spades, notrumps };
>
> and a function
>
> int b(int level, int suit) { return 15 * level + 3 * suit; }

Here you clearly want to have constexpr function.

>
> which allow me to use for instance either B_1NT or b(1, notrumps) as the value for a one notrumps bid.
>
> Part of me thinks I ought to update the first enum to be more like
>
> enum { B_1NT = b(1, notrumps) }
>
> but this does not work, even if I make b inline.
>

You want constexpr, not inline. The constexpr implies inline.

> Presumably if would work if I made b a macro, but these are generally discouraged. I'm guessing that some syntax using constexpr might work (I've never used one of them before) but would I need to include the code for b twice, once for the constants and once as a normal function?

You mix up constexpr and consteval here. The consteval function must run
compile time but constexpr can be ran run time as well.

[Paul N]

Thanks Paavo, that's just the job. I didn't realise constexpr could handle both compile-time and non-compile-time values.

[Paul N]

On Tuesday, January 17, 2023 at 1:23:47 PM UTC, Öö Tiib wrote:
> On Tuesday, 17 January 2023 at 14:29:51 UTC+2, Paul N wrote:
> > I'm attempting to write a Bridge program, and it's getting quite messy.
> >
> > At first I simply used a number for each bid, so my code had a lot of "magic numbers" in it. I got rid of some by using an enum, as follows:
> >
> > enum { B_PASS = 0, B_DOUBLE = 1, B_REDOUBLE = 2,
> > B_1C = 15, B_1D = 18, B_1H = 21, B_1S = 24, B_1NT = 27,
> > B_2C = 30, B_2D = 33, B_2H = 36, B_2S = 39, B_2NT = 42,
> > B_3C = 45, B_3D = 48, B_3H = 51, B_3S = 54, B_3NT = 57,
> > B_4C = 60, B_4D = 63, B_4H = 66, B_4S = 69, B_4NT = 72 };
> >
> Nameless type, SCREAMING CAPS names and then using just ints anyway?

Well, it's nearly a macro, isn't it? If you have any better ideas, I'd (seriously) be glad to hear them, that's the whole point of this post.

The values aren't random, they also represent contracts, with 16 representing 1 clubs doubled, 17 representing 1 clubs redoubled, etc.

> > This lets me do code such as
> >
> > if (WentMP(B_1NT, B_2NT)) { // rebid after 1NT - 2NT
> > if (pc == 14 || (pc == 13 && tens >= 2)) { mess = _TEXT("Raise"); bid = B_3NT; }
> > else { mess = _TEXT("Not got 25 points"); } }
> >
> > which is relatively clear.
> >
> > However, now that I'm trying to deal with suit bids as well it seems silly to deal with each bid separately. I have an enum
> >
> > enum { clubs, diamonds, hearts, spades, notrumps };
> >
> > and a function
> >
> > int b(int level, int suit) { return 15 * level + 3 * suit; }
> Here you clearly want to have constexpr function.

Yes, Paavo said that too and gave me the syntax. I perhaps ought to keep up more with these recent (!) developments to the language.

> > which allow me to use for instance either B_1NT or b(1, notrumps) as the value for a one notrumps bid.
> >
> > Part of me thinks I ought to update the first enum to be more like
> >
> > enum { B_1NT = b(1, notrumps) }
> >
> > but this does not work, even if I make b inline.
> >
> You want constexpr, not inline. The constexpr implies inline.
> > Presumably if would work if I made b a macro, but these are generally discouraged. I'm guessing that some syntax using constexpr might work (I've never used one of them before) but would I need to include the code for b twice, once for the constants and once as a normal function?
> You mix up constexpr and consteval here. The consteval function must run
> compile time but constexpr can be ran run time as well.

Thanks for the explanation. I'd heard of constexpr (but didn't know much about it) whereas I'd not heard of consteval.

[David Brown]

On 17/01/2023 14:52, Paul N wrote:
> On Tuesday, January 17, 2023 at 1:23:47 PM UTC, Öö Tiib wrote:
>> On Tuesday, 17 January 2023 at 14:29:51 UTC+2, Paul N wrote:
>>> I'm attempting to write a Bridge program, and it's getting quite messy.
>>>
>>> At first I simply used a number for each bid, so my code had a lot of "magic numbers" in it. I got rid of some by using an enum, as follows:
>>>
>>> enum { B_PASS = 0, B_DOUBLE = 1, B_REDOUBLE = 2,
>>> B_1C = 15, B_1D = 18, B_1H = 21, B_1S = 24, B_1NT = 27,
>>> B_2C = 30, B_2D = 33, B_2H = 36, B_2S = 39, B_2NT = 42,
>>> B_3C = 45, B_3D = 48, B_3H = 51, B_3S = 54, B_3NT = 57,
>>> B_4C = 60, B_4D = 63, B_4H = 66, B_4S = 69, B_4NT = 72 };
>>>
>> Nameless type, SCREAMING CAPS names and then using just ints anyway?
>
> Well, it's nearly a macro, isn't it? If you have any better ideas,
> I'd (seriously) be glad to hear them, that's the whole point of this
> post.
>

Macro names don't need all caps. I only ever use all-caps names if I
have a macro that is doing something weird, and you have to pay special
attention to it. So if I have a function-like macro that behaves very
much like a function, it does not need all-caps in the name. (Of
course, in C++ this generally would be a real function - it's more
common in C, when you need something that is type generic and you don't
have templates.) Similarly, macros that are simple constants do not
need all-caps names. (And again, there are usually better choices than
macros.)

If the macro evaluates a parameter more than once, or plays silly
buggers with scopes, declares new variables, etc., in a way that is not
obvious in its use, then all-caps is a good warning. But overuse of a
warning negates its effectiveness.

> The values aren't random, they also represent contracts, with 16
> representing 1 clubs doubled, 17 representing 1 clubs redoubled,
> etc.
>

Then don't define them this way. Do it using a constexpr function, as
discussed in other posts, or by some completely different arrangement
(such as a const template variable, or a class that wraps the value
while storing it in a bitfield struct - there are a number of options).
Whenever there are hand-calculated magic numbers, you should be
sceptical. It /might/ be the simplest and clearest way to write the
code, but it might not be. And it's seldom the most /fun/ way to write
the code!

[Paul N]

Thanks for the comments, David. My first attempt at the code used all the magic numbers and so having an enum was at least an improvement on that. Now that I have the function, I have more options, but it's still not entirely clear to me whether changing code from the already written bid = B_3NT; to the slightly longer bid = b(3, notrumps) is really an improvement. Hence the canvassing of opinions.

[Bo Persson]

On 2023-01-17 at 14:52, Paul N wrote:
> On Tuesday, January 17, 2023 at 1:23:47 PM UTC, Öö Tiib wrote:
>> On Tuesday, 17 January 2023 at 14:29:51 UTC+2, Paul N wrote:
>>>
>>> and a function
>>>
>>> int b(int level, int suit) { return 15 * level + 3 * suit; }
>>
>> Here you clearly want to have constexpr function.
>
> Yes, Paavo said that too and gave me the syntax. I perhaps ought to keep up more with these recent (!) developments to the language.
>

Yes, as recent as 2011. :-)

[Richard Damon]

I might change the encoding of the enum, thinking of it a bit like a
bitfield.

Bottom 2 bits is the "Double" status: 01 Undoubled, 10 Doubled, 11
Redoubled (this lets all zeros be the "pass" option"

Next 3 bits are the suit 000 = Clubs, 001 = Diamonds, 010 = Hearts, 011
= Spades, 100 = No Trump

Top 3 bits are the contract: 001 = 1 bid .... 111 = 7 bid.

So actual bits go from 1 Clubs Undoubled = 001 000 01 = 33 decimal to
7 NT Redoubled = 111 100 11 = 243

Perhaps even have 3 seperate enums for each of these and since you are
using C++ define a constexpr operator that lets you merge them together.
(you will need another enum type for Contract + Suit to build the full
bids up.)

You can also define conversion functions to get you back to the
components (which just need to do bit masking).