0

Hello I didn't use C++ for years now, so I'm kinda rusty and can't figure this out!

I have this code | person.h

#ifndef PERSON_H
#define PERSON_H

#include<string>

class Person {
    private:
        std::string fName, lName;
        int age;
        enum professionEnum {
            Barber,
            Developer,
            Marketer
        } profession;

    public:
        Person();
        Person(std::string fName, std::string lName, int profession);
        void setFName(std::string fName);
        void print();

};

#endif

a simple person class.

What I want to do is for my constructor Person() to take an int, example :

Person me = Person("me", "meow", 0); // This should assign "me" to Barber, enum index is 0.

Here is what I tried so far (all errors):

this->profession = professionEnum::profession;
this->profession = 0;
this->profession = professionEnum(profession);

Here is the full person.cpp code

#include "person.h"

#include <iostream>

Person::Person() {
    this->age = 18;
    this->fName = "Default";
    this->lName = "Default";
}

Person::Person(std::string fName, std::string lName, int profession) {
    this->fName = fName;
    this->lName = lName;
    this->profession = professionEnum::profession;
    this->profession = 0;
    this->profession = professionEnum(profession);
}

void Person::print() {
    std::cout << "Age : " << this->age << ", FName : " << this->fName << ", LName : " << this->lName << std::endl;
}

Thanks for your help

PS: If you spot any bad practices / bad code, please tell me.

pmg
  • 106,608
  • 13
  • 126
  • 198
0xRyN
  • 852
  • 2
  • 13
  • You could make the `professionEnum ` public, and pass that type to the constructor instead of int. To cast you need to use `profession = (professionEnum) profession_int` (or, more properly, `profession = static_cast(profession_int)` You shouldn't need to use `this->` all over the place. – Den-Jason Sep 10 '21 at 11:29
  • If you cast the int to the enum it works here: https://ideone.com/xcJLu7 – Tommy Andersen Sep 10 '21 at 11:30
  • 1
    instead of `enum` prefer `enum class` – macroland Sep 10 '21 at 11:30
  • Your 3rd form `this->profession = professionEnum(profession);` works on Visual Studio 16.9.5 with c++latest (more-or-less c++20, yes I'm not fully updated, reasons, don't come at me bro). So I'm not seeing the error you're seeing. You should use `static_cast` and probably `enum class` but I do not see a compiler error. – Kevin Anderson Sep 10 '21 at 11:31
  • 1
    also instead of `#ifndef` prefer `#pragma once`, more concise – macroland Sep 10 '21 at 11:33
  • 1
    Why is the thing that could provide the user with a little convenience, and the interface with a minimum of type safety (i.e. the enum), private and not used in the interface? – molbdnilo Sep 10 '21 at 11:43
  • 1
    @macroland `#pragma once` is a compiler extension, no? – JohnFilleau Sep 10 '21 at 11:47
  • 1
    @molbdnilo -- `#pragma once` is not standard C++. – Pete Becker Sep 10 '21 at 12:30
  • @macroland • Zack Weinberg recommends against using `#pragma once` ([link](https://stackoverflow.com/a/34884735/4641116)). Zack is the creator of `#pragma once`. – Eljay Sep 10 '21 at 13:45
  • https://en.wikipedia.org/wiki/Pragma_once is not standard but supported by many compilers. And regarding the use please see this discussion: https://www.reddit.com/r/cpp/comments/4cjjwe/come_on_guys_put_pragma_once_in_the_standard/d1j04te/ – macroland Sep 10 '21 at 15:10
  • @molbdnilo What do you mean ? Should I put the enum public ? – 0xRyN Sep 10 '21 at 22:13
  • @macroland can you tell me why ? – 0xRyN Sep 10 '21 at 22:13
  • @TommyAndersen I have these errors on this : (sorry formatting) /Users/rayan/Documents/Dev/C++/Playground/src/person.cpp:14:24: error: use of undeclared identifier 'professionEnum' this->profession = professionEnum(profession); ^ /Users/rayan/Documents/Dev/C++/Playground/src/person.cpp:18:37: error: cannot refer to type member 'profession' in 'Person' with '->' std::cout << "Prof : " << this->profession << ", FName : " << this->fName << ", LName : " << this->lName << std::endl; – 0xRyN Sep 10 '21 at 22:15
  • @Den-Jason thank you. For the this->, what do you recommend instead ? – 0xRyN Sep 10 '21 at 22:18
  • 1
    @RayanDev Yes, you should. That is, the enum *type* should be public, not the member variable. `Person figaro("First", "Last", Person::Barber)` looks pretty useful, no? And if you rearrange the enum values, it will keep working. – molbdnilo Sep 11 '21 at 07:39
  • @RayanDev in most cases, within the class methods you can omit `this->` completely since it is implied. Don't use parameter names that are the same as class attributes. – Den-Jason Sep 11 '21 at 09:03

0 Answers0