3
\$\begingroup\$

I'm a newbie in C++ (6 hours) and I have implemented this problem in C++ . I know this is a simple example but how can this be improved? Syntax wise or shortened code wise?

Here is the question from Hackerrank

Create three classes Person, Professor and Student. The class Person should have data members name and age. The classes Professor and Student should inherit from the class Person.

The class Professor should have two data members: publications and cur_{id}. There will be two member functions: getdata and putdata. The function getdata should get the input from the user: the name, age and publications of the professor. The function putdata should print the name, age, publications and the cur_{id} of the professor.

The class Student should have two data members: marks, which is an array of size and cur_{id}. It has two member functions: getdata and putdata. The function getdata should get the input from the user: the name, age, and the marks of the student in subjects. The function putdata should print the name, age, sum of the marks and the cur_{id} of the student.

For each object being created of the Professor or the Student class, sequential id's should be assigned to them starting from \$1\$.

Solve this problem using virtual functions, constructors and static variables. You can create more data members if you want.

Input Format

There are two types of input. If the object being created is of the Professor class, you will have to input the name, age and publications of the professor.

If the object is of the Student class, you will have to input the name, age and the marks of the student in \$6\$ subjects.

  • \$ 1 <= len_{name} <=100 \$ where \$len_{name}\$ is the length of the name
  • \$ 1<= age <= 80\$
  • \$ 1 <= publications <=1000 \$
  • \$ 0 <= marks<=100 \$, where marks are the marks of the student in each subject

Output Format

There are two types of output depending on the object.

If the object is of type Professor, print the space separated name, age, publications and id on a new line.

If the object is of the Student class, print the space separated name, age, the sum of the marks in subjects and id on a new line.

Sample Input

4
1
Walter 56 99
2
Jesse 18 50 48 97 76 34 98
2
Pinkman 22 10 12 0 18 45 50
1
White 58 87

Sample Output

Walter 56 99 1
Jesse 18 403 1
Pinkman 22 135 2
White 58 87 2

Here is my solution

#include <cmath>
#include <cstdio>
#include <vector>
#include <iostream>
#include <algorithm>
using namespace std;
class Person{
    public:
         string name;
    public:
         int age;
    virtual void getdata(){}
    virtual void putdata(){}
    virtual int  getID(){return 0;}
};

class Professor: public Person{

    int publications; 
    static int id;
    int getID(){return id;}
    public:
      void getdata () { 
          cin >> name;
          cin >> age;
          cin >> publications;
      }

    public:
        void putdata(){
             id++;
             cout << name + " "<< age << " "<<publications<<" "<<getID()<<"\n";
        }

};
int Professor::id = 0;

class Student: public Person{

    float marks[6];
    static int id;
    int getID(){return id;}
    public:
      void getdata () { 
          cin >> name;
          cin >> age;
          for(int i =0 ; i< 6; i++){
              cin >> marks[i];
          }
      }

      public:
        void putdata(){
             id++;
             float markSum = 0;
            for(int i =0 ; i < 6; i++){
                markSum += marks[i];
            }
            cout << name + " "<< age << " "<<markSum<<" "<<getID()<<"\n";
        }        
};
int Student::id = 0;

NB: The main() was already implemented and I had no access to change it but the classes were implemented by me.

int main(){

    int n, val;
    cin>>n; //The number of objects that is going to be created.
    Person *per[n];    
    for(int i = 0;i < n;i++){    
        cin>>val;
        if(val == 1){
            // If val is 1 current object is of type Professor
            per[i] = new Professor;    
        }
        else per[i] = new Student; // Else the current object is of type Student   
        per[i]->getdata(); // Get the data from the user.    
    }

    for(int i=0;i<n;i++)
        per[i]->putdata(); // Print the required output for each object.  

    return 0;    
}
\$\endgroup\$
4
\$\begingroup\$
#include <cmath>
#include <cstdio>
#include <vector>
#include <iostream>
#include <algorithm>

Do you really need all of that? Especially mixing <iostream> and <cstdio> seems questionable. But I would reduce on that and use only

#include <iostream>
#include <string>
using namespace std;

Please, don't do that. It's considered bad practice for various reasons and will only save you 5 characters every once in a while. Using 20 in the process to do so…

class Person{
    public:
         string name;
    public:
         int age;
    virtual void getdata(){}
    virtual void putdata(){}
    virtual int  getID(){return 0;}
};

First off, the descriptions didn't ask for any method in this class. But I would keep two of them (the ones not dealing with IDs, that are not part of this class). Second, you don't need to put several visibility marker: once you write public:, everything that follows is public. Until an other visibility is used. Third, keep those attributes protected: they are your internals, you don't want every one to mess with them; at least not in ways you didn't allowed through your public interface. Lastly, since they don't have any code, I would turn the methods into pure virtual ones:

class Person{
    protected:
         std::string name;
         int age;

    public:
        virtual void getdata() = 0;
        virtual void putdata() = 0;
};
class Professor: public Person{
    int publications; 
    static int id;
    int getID(){return id;}
    public:
      void getdata () { 
          cin >> name;
          cin >> age;
          cin >> publications;
      }
    public:
        void putdata(){
             id++;
             cout << name + " "<< age << " "<<publications<<" "<<getID()<<"\n";
        }
};
int Professor::id = 0;

Same here, you don't need public all over the code. Only once suffice. You are also using the static int id as the cur_id the challenge ask for. This is a mistake: if I create a Professor and output its informations using:

Professor p;
p.getdata();  // Inputting `Alice 42 42`
p.putdata();
p.putdata();

I will see printed:

Alice 42 42 1
Alice 42 42 2

Your code only works because there is only one putdata on each object in the right order. Any other usage would not work as intended.

You also appear to mix string concatenation and stream output to put a space between elements: get consistent. You can also chain the stream operator >> to condense the code a bit without loosing on readability:

class Professor: public Person {
    static int id;

    protected:
        int publications; 
        int current_id;

    public:
        void getdata () { 
            std::cin >> name >> age >> publication;
            current_id = ++id;
        }

        void putdata() {
             std::cout
                 << name << " " << age << " " << publications
                 << " " << current_id << "\n";
        }
};

int Professor::id = 0;
class Student: public Person{
    float marks[6];
    static int id;
    int getID(){return id;}
    public:
      void getdata () { 
          cin >> name;
          cin >> age;
          for(int i =0 ; i< 6; i++){
              cin >> marks[i];
          }
      }
      public:
        void putdata(){
             id++;
             float markSum = 0;
            for(int i =0 ; i < 6; i++){
                markSum += marks[i];
            }
            cout << name + " "<< age << " "<<markSum<<" "<<getID()<<"\n";
        }        
};
int Student::id = 0;

Appart from remarks I already said for Professor, I was surprised to see that you store the marks as floats. Nothing in the text nor in the example input lead me to such conclusion, I would expect ints here.

class Student: public Person {
    static int id;

    protected:
        int marks[6];
        int current_id;

    public:
        void getdata () { 
            std::cin >> name >> age;
            for(int i = 0; i < 6; ++i) {
                std::cin >> marks[i];
            }
            current_id = ++id;
        }

        void putdata() {
            int markSum = 0;
            for(int i = 0; i < 6; ++i){
                markSum += marks[i];
            }
            std::cout
                << name << " " << age << " "
                << markSum << " " << current_id << "\n";
        }        
};

int Student::id = 0;

One thing that I didn't enforce here but that you should get into the habit of using is qualifying attribute access using this-> (such as this->name or this->current_id). It does not matter much here as the code is fairly simple but it can help a lot to understand things more easily when the code is more consequent.

Lastly, reading at the parameters ranges, you can consider using unsigned ints rather than ints.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.