How do I correctly use enum within a class?

Question

I am trying to set the status of an object from a private enum and I'm not sure how to get the private member to the public version in the constructor, and I'm pretty sure I should not be using byte in the constructor when it's an enum.

Also, I'm not sure if I should even have the values of the enum inside the header at all. Here is my code:

.h

class CandleRack
{
 public:
 CandleRack(byte candleRackStatus);
 ~CandleRack();
 void begin();
 void pushButton();
 void selectRandomCandle();
 void turnOnCandle(byte candlePosition);
 void burnCandlesForMs();
 void fadeOutCandle(byte candlePosition);
 private:
 Bounce *_debouncePushButton;
 byte _pushButtonPin; 
 enum _candleRackStatus
 {
 inactive = 0,
 candlesOn = 1,
 rewardAnimation = 2,
 tweetArrived = 3,
 partyMode = 4, 
 };
 byte _candlePosition;
 byte _candleStatus[NUM_LEDS];
 elapsedMillis _candleTimeElapsed[NUM_LEDS];
 unsigned int _candleOnForMs;
};

.cpp

CandleRack::CandleRack(byte candleRackStatus)
{
 _candleRackStatus = candleRackStatus;
 _debouncePushButton = NULL;
}

in the sketch

CandleRack MyCandleRack(0);

error:

error: expected unqualified-id before '=' token
_candleRackStatus = candleRackStatus;

Thanks!

• aaa: Might be more of a programming question as an Arduino question. I've heard that ENUM's are specifically useful when dealing with switch-case statements, since you can define a "default" case.
• Rob Hilken: You can define a 'default' case on a switch statement without needing an enum, but for me it will help the code be more usable as you can give each case a name and not just a number.
• Steve: SHouldn't CandleRack MyCandleRack(0); be CandleRack MyCandleRack(inactive); or maybe CandleRack::inactive. My C++ is a bit rusty (very rusty). Also, in the interests of program readability the enum would be useful outside the class otherwise you're just assigning a bunch of numbers.
• Rob Hilken: The enum gives the object its properties so my logic tells me it should be a part of the class, it's also a private member so it's properties are hidden from all of the other classes I will be adding. You're right about MyCandleRack(inactive); though.
• aaa: Hmm, you're true on the default thing. But I can vaguely remember my prof being "mad" at me for using defines instead of enums, since your compiler could check if all enums are handled in your switch-case.

Answer 1

I see two separate issues here.

First of all your enum isn't available as a variable. By declaring it as a private type within your class the enum can't be used outside of that class.

This forces a user of that class to handle unintended calls like (attention, bug):

CandleRack(255);

which can make code harder to read (and debugging more difficult, since it wouldn't be detected by the compiler).

Second of all you try to directly assign a byte value to an enum type. This will be rejected by the compiler and/or cause runtime errors later (depending on compiler settings).

I recommend that you declare the enum type so that it is accessible directly via the header file:

.h

enum CandleRackStatus {
 inactive = 0,
 candlesOn = 1,
 rewardAnimation = 2,
 tweetArrived = 3,
 partyMode = 4, 
};
class CandleRack
{
 public:
 CandleRack(CandleRackStatus candleRackStatus);
 ~CandleRack();
 void begin();
 void pushButton();
 void selectRandomCandle();
 void turnOnCandle(byte candlePosition);
 void burnCandlesForMs();
 void fadeOutCandle(byte candlePosition);
 private:
 Bounce *_debouncePushButton;
 CandleRackStatus _candleRackStatus;
 byte _pushButtonPin; 
 byte _candlePosition;
 byte _candleStatus[NUM_LEDS];
 elapsedMillis _candleTimeElapsed[NUM_LEDS];
 unsigned int _candleOnForMs;
};

Now you can instantiate your class while using the correct enum type as the parameter.

.ino

CandleRack MyCandleRack(CandleRackStatus::inactive);

Within your implementation you can now directly store the constructor parameter in the private variable, the compiler will make sure it's always of the correct enum type:

.cpp

CandleRack::CandleRack(CandleRackStatus candleRackStatus) {
 _candleRackStatus = candleRackStatus;
 // rest of your initialisiation goes here
}

As an added bonus, there's the possibility to use a couple of C++ language features that will make your implementation easier:

• a default parameter value
• an initializer (list)

.h

class 
{
 public:
 CandleRack(CandleRackStatus candleRackStatus = CandleRackStatus::inactive);
 // [...]
}

.cpp

CandleRack::CandleRack(CandleRackStatus candleRackStatus) : _candleRackStatus(candleRackStatus) {
 // rest of your initializiation goes here
}

• Rob Hilken: One thing I don't understand is why you have Capitalized the name CandleRackStatus. I thought an enum was just a variable and nothing like a class or struct. I think it's confusing to treat it like a class (with naming conventions) ...
• Rob Hilken: last question, when I actually want to change the status of the CandleRack, do I do _candleRackStatus = candlesOn ?
• sekdiy: Re: Capitalized, that's a matter of taste and what school of programming you come from. I recommend treating enums like types, because they are. And in most C++ styles types get a capitalized name. See google.github.io/styleguide/cppguide.html#Enumerator_Names for reference, also a good read as a whole.
• sekdiy: Re: changing the status, the identifier candlesOn is defined within the type declaration of the enum. So in order to access it you would qualify it with the type name: CandleRackStatus::candlesOn. That's called static access and the :: operator works on all type properties that are public and static (see stackoverflow.com/questions/10090949/…). So you would change the status using _candleRackStatus = CandleRackStatus::candlesOn.
• sekdiy: We've discussed simplification of names before, so if these expressions are too lengthy or confusing, just choose shorter names for types and variables.

Answer 2

How about this refactoring?

class CandleRack
{
 public:
 enum status_t
 {
 inactive = 0,
 candlesOn = 1,
 rewardAnimation = 2,
 tweetArrived = 3,
 partyMode = 4, 
 }; 
 CandleRack(status_t status = inactive);
 ~CandleRack();
 void begin();
 void pushButton();
 void selectRandomCandle();
 void turnOnCandle(byte candlePosition);
 void burnCandlesForMs();
 void fadeOutCandle(byte candlePosition);
 private:
 Bounce *_debouncePushButton;
 byte _pushButtonPin;
 status_t _candleRackStatus;
 byte _candlePosition;
 byte _candleStatus[NUM_LEDS];
 elapsedMillis _candleTimeElapsed[NUM_LEDS];
 unsigned int _candleOnForMs;
};

And in a sketch with either the default setting (inactive)

CandleRack MyCandleRack;

or explicit

CandleRack MyCandleRack(CandleRack::candlesOn);

Cheers!

• sekdiy: It seems this answer compiles the gist of my answer into a code-only post (see e.g. meta.stackexchange.com/questions/148272/…)
• Rob Hilken: Yes It does a bit. If you could add some of these suggestions to your post I think that would make it a great answer. Especially as this shows how I could use the enum as a public member and not outside of the class - which in my mind, makes it more understandable.
• Rob Hilken: But also thanks to Mikael for this post too, it also helps my understanding of how to use enums.
• Rob Hilken: In this example where is status coming from? status_t status = inactive
• sekdiy: status is an alternative naming to your candleRackStatus. Simpler names can increase readability. Since you only pass one parameter, you don't really need to qualify it in more detail than neccessary, so it's probably a good idea to simplify. :) I've updated my answer to explain the use of the enum type and a default parameter value.