I am trying to acquaint myself with the use/abuse of classes and have hit a snag
Here is my test sketch
class train
{
private:
char *_name;
public:
train() //constructor - no return type
{
}
void setName(char *name)
{
strcpy(_name, name); //save the name
}
char *getName()
{
return _name;
}
};
train redTrain(); //instance of train object named redTrain
void setup()
{
Serial.begin(115200);
redTrain.setName("red"); //give redTrain a name
Serial.printf("Name : %s\n", redTrain.getName()); //print the name of redTrain
}
void loop()
{
}
This produces errors as follows
C:\Users\micro\Documents\Arduino\train_class_1\train_class_1.ino: In function 'void setup()':
C:\Users\micro\Documents\Arduino\train_class_1\train_class_1.ino:27:14: error: request for member 'setName' in 'redTrain', which is of non-class type 'train()'
27 |
| ^
C:\Users\micro\Documents\Arduino\train_class_1\train_class_1.ino:28:43: error: request for member 'getName' in 'redTrain', which is of non-class type 'train()'
28 | redTrain.init("red");
| ^
exit status 1
Compilation error: request for member 'setName' in 'redTrain', which is of non-class type 'train()'
This is using IDE 2.3.3 on Windows 11 with EP32 board files version 3.05 and ESP32 Dev Module as the target board. Reverting to earlier board files makes no difference
Not an answer to your problem. But this does not reserve space to store the name. So you cannot strcopy something in this place.
You either need to hand a pointer to a piece of memory that will exist throughout the lifetime of your object, or you need to new() space, or you need to reserve a fixed char array.
"red" is a string literal. Not a char*.
Iso c++ somenumber forbids handing a pointer to a string literal...
It might or might not work, depending on your particular board architecture...
Got it working. The error message was misleading because the cause of the problem was the misuse of the char *
class train
{
private:
char _name[20];
public:
train() //constructor - no return type
{
}
void setName(char name[20])
{
strcpy(_name, name); //save the name
}
char *getName()
{
return _name;
}
};
train redTrain; //instance of train object named redTrain
void setup()
{
Serial.begin(115200);
redTrain.setName((char *)"red"); //give redTrain a name
Serial.printf("Name : %s\n", redTrain.getName()); //print the name of redTrain
redTrain.setName((char *)"now RED"); //give redTrain a name
Serial.printf("Name : %s\n", redTrain.getName()); //print the name of redTrain
}
void loop()
{
}
or the alternative version
class train
{
private:
char _name[20];
public:
train() //constructor - no return type
{
}
void setName(char name[20])
{
strcpy(_name, name); //save the name
}
char *getName()
{
return _name;
}
};
train redTrain; //instance of train object named redTrain
void setup()
{
Serial.begin(115200);
char name[20];
strcpy(name, "name 1");
redTrain.setName(name); //give redTrain a name
Serial.printf("Name : %s\n", redTrain.getName()); //print the name of redTrain
strcpy(name, "name 2");
redTrain.setName(name); //give redTrain a name
Serial.printf("Name : %s\n", redTrain.getName()); //print the name of redTrain
}
void loop()
{
}
Your setName() function doesn't need to modify the source string. So, promise the compiler that you won't try to. Then get rid of the casts when you call it:
class train {
private:
char _name[20];
public:
train() {} //constructor - no return type
void setName(const char *name) {
strncpy(_name, name, 19); //save the name
_name[19] = '\0';
}
char *getName(){
return _name;
}
};
train redTrain; //instance of train object named redTrain
void setup() {
Serial.begin(115200);
redTrain.setName("red"); //give redTrain a name
Serial.printf("Name : %s\n", redTrain.getName()); //print the name of redTrain
redTrain.setName("now RED"); //give redTrain a name
Serial.printf("Name : %s\n", redTrain.getName()); //print the name of redTrain
}
void loop() {
}
if you decide tou want to keep a local copy instead than a pointer to the original cString , I would recommend to use strncpy() or strlcpy() to not overflow your buffer size.
change
into
strlcpy(_name, name, sizeof _name); //save the name
Also I would advise against using _name for members. A frequent C++ naming convention to adopt is to use m_xxx for member variables but it diverges from the camelCase we are used to in Ardino's world so my recommendation would be to juste go for name and drop the underscore altogether.
Also I would capitalise the class name (Train instead of train)
As you constructor does not do anything, just don't write it. The C++ language guarantees that you'll get an implicit default constructor doing what you need (or alternatively pass the name to the constructor).
class Train {
private:
const char * name;
public:
Train(const char * name) : name(name) {}// constructor with initializer list
const char *getName() {
return name;
}
};
Train trains[] = {"red", "blue", "black"};
const byte trainCnt = sizeof trains / sizeof * trains;
void setup() {
Serial.begin(115200);
// range for
for (auto &aTrain : trains) {
Serial.println(aTrain.getName());
}
// or traditional for with an index
for (byte i = 0; i < trainCnt; i++) {
Serial.print("Train #");
Serial.print(i);
Serial.print(": ");
Serial.println(trains[i].getName());
}
}
void loop() {}
I wasn't yet addressing that part of the code, just the cause of the error message. But yea, it should be const char *getName() {. The other const is not needed as the pointer itself is passed by value.
I have moved the sketch on quite a bit since getting past the original error, to the extent that I now have an array of Train (uppercase T @J-M-L ) objects each with several more member variables
Is there a specific reason for this ?
Some time ago when I first tinkered with classes the advice was to give private variables names preceded with an underscore, not because of any magic that it endowed them with, but as a reminder to the programmer that they were private member variables
Because your original getName() returned a pointer to name. That returned pointer could be used to modify name without the class's knowledge. Per Post #9, return a const char *,
Then you might as well use a String class object rather than allocating the storage yourself. That guarantees that the storage will be freed when the containing object is destroyed, even if the author forgets to do so in their class's destructor.
In C++, identifiers starting with an underscore followed by an uppercase letter or containing double underscores are reserved for the implementation and should not be used by programmers.
Identifiers that start with an underscore followed by a lowercase letter are reserved at the global scope.
A member variable of your class is in the local scope, so the rule does not apply, but to avoid confusion and potential conflicts (if inadvertently used outside local scope), many C++ style guides suggest avoiding leading underscores entirely and you'll often see m_xxx being proposed if you want a special naming convention.
you can write a constructor without parameters and one with the name
our style required the use of the underscore prefixes for static variables and function names within a file (not a class). such symbols would not be accessible outside file scope
i think a common motivations for underscore in classes is initialization in a constructor with similar names
No more so than allocating dynamic storage yourself (and possibly forgetting to free it at the correct time).
Not if it's a local variable in a function.
Yes, as I mentioned, if the author remembers to do so. Given that a debugged / tested class already exists to handle dynamic allocation / destruction of character strings, it's safer to use it.
