2

I am trying to insert a set of pair values into a std::map in . However, the values don't seem to insert into the std::map. Please do go over my code about the same. I appreciate any and all help.

#include<iostream>
#include<string>
#include<algorithm>
#include<vector>
#include<map>
#include<cstdlib>
#include<utility>
#include<ctime>

#include "print.h"

class ReportCard
{
private:
    std::map<std::string, double> m_report_card;

public:
    std::map<std::string, double> getReportCardInstance() {  return m_report_card;  }
};

class Student
{

private:
    int m_roll_no;
    std::string m_name;
    ReportCard m_reportCard;

public:
    Student(int inRollNo, const std::string& inName) :
        m_roll_no(inRollNo), m_name(inName)
    {}

    std::string getName()   {   return m_name;  } 
    int getRollNo()     {   return m_roll_no;   }
    ReportCard getReportCard()  {   return self.m_reportCard;   }
    int getReportCardSize() {   return m_reportCard.getReportCardInstance().size(); }
};

class Driver
{
private:
    std::vector<Student> student_list;
    std::vector<Student> temp;

public:
    void studentTestPopulate()
    {
        student_list.push_back(Student(1, "Tim"));
        student_list.push_back(Student(2, "Matt"));
        student_list.push_back(Student(100, "Luke"));
        student_list.push_back(Student(68, "Lissy"));
        student_list.push_back(Student(20, "Tony"));
        student_list.push_back(Student(33, "Joseph"));
        student_list.push_back(Student(14, "Sid"));
        student_list.push_back(Student(15, "Roby"));
        student_list.push_back(Student(44, "Rohan"));
        student_list.push_back(Student(11, "Kevin"));
        student_list.push_back(Student(19, "George"));
    }
    void reportCardPopulate()
    {
        for (auto& student : student_list)
        {
            std::cout << student.getName() << std::endl;
            student.getReportCard().getReportCardInstance().insert(std::make_pair<std::string, double>("Math", generateMark));
            //This is the function that does not work. No marks are printed!!
            for (auto& mark : student.getReportCard().getReportCardInstance())
            {
                std::cout << mark.first << " " << mark.second;
            }
            //student.getReportCard().getReportCardInstance().insert(std::make_pair("Science", generateMark));
            //student.getReportCard().getReportCardInstance().insert(std::make_pair("Geography", generateMark));
            //student.getReportCard().getReportCardInstance().insert(std::make_pair("French", generateMark));
            //student.getReportCard().getReportCardInstance().insert(std::make_pair("History", generateMark));
        }
    }
    void showAllStudentDetails()
    {
        for (auto& student : student_list)
        {
            std::cout << student.getName() << std::endl;
            std::cout << student.getRollNo() << std::endl;
            std::cout << "REPORT CARD : " << student.getReportCardSize() << std::endl << std::endl;
            for (auto& mark : student.getReportCard().getReportCardInstance())
            {
                std::cout << mark.first << std::endl;
                std::cout << mark.second << std::endl;
            }
        }
    }
};

int main()
{
    srand(time(NULL));
    Driver driver;
    driver.studentTestPopulate();
    driver.reportCardPopulate();
    //driver.showAllStudentDetails();
}

The reportCardPopulate() function is supposed to insert pairs of values into a report_card map. However, the insert function doesn't seem to work.

When we try to print the values within the reportCardPopulate() function, it doesn't print anything. When I try to print the size of the map, it prints 0. When I printed the size using sizeof() it prints the same size before and after the insertion.

JeJo
  • 30,635
  • 6
  • 49
  • 88
james
  • 131
  • 9
  • 2
    `Please do go over my code` Please minimize your code. –  Aug 28 '19 at 23:59
  • 1
    `ReportCard getReportCard()` and `std::map getReportCardInstance()` return a copy where you most likely want a reference or perhaps a different interface so you don't expose class internals. – Retired Ninja Aug 29 '19 at 00:01

2 Answers2

2
#include <iostream>
#include <map>

class ReportCard
{
   //private:  this is the default anyway for a class
   public: //made to be able to print the internals below.
        std::map<std::string, double> m_report_card;

   public:


        /* this returns an instance of the std::map. The map is copied and 
        returned, so any modifications will not affect m_report_card
        std::map<std::string, double> getReportCardInstance()
        {
            return m_report_card;
        }

        if you want to do this, return std::map<std::string, double>&.
        std::map<std::string, double>& getReportCardInstance()
        {
            return m_report_card;
        }
        */

        // better solution is to have a method to add the report

        void add_report(const std::string& first,double second)
        {
            m_report_card[first] = second;
        }  

};


int main() {
    ReportCard rc;
    rc.add_report("Percy",1.0);
    rc.add_report("Pig",2.0);

    for(auto internal_report_card : rc.m_report_card)
    {
        std::cout << internal_report_card.first << ", " 
                  << internal_report_card.second << std::endl;
    }

    return 0;
}

Demo

rmawatson
  • 1,909
  • 12
  • 20
2

The following functions

std::map<std::string, double> getReportCardInstance() { ... }
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ReportCard getReportCard() { ... }
//^^^^^^^^

returns the copy of std::map<std::string, double> and ReportCard class respectively. Therefore, whatever you insert here

student.getReportCard().getReportCardInstance().insert(std::make_pair<std::string, double>("Math", generateMark));

does on the copies of the above, hence the original member in ReportCard(i.e. m_report_card) will never get be updated. After the call of above line, the copies will be destroyed and expecting it to work make no sense.

Secondly, shown code is wrong, because in c++ you should have used this not self

ReportCard getReportCard()
{
    return self.m_reportCard;
         //^^^^ --> should be `return this->m_reportCard;`
         // or simply         `return m_reportCard;`
}

Correcting the above, and returning the member by reference will make the code work. (See live online)

std::map<std::string, double>& getReportCardInstance()
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^
{
    return m_report_card;
}

ReportCard& getReportCard()
//^^^^^^^^
{
    return m_reportCard;
}

That being said, your ReportCard and Student classes will expose the members, if you do the above. Which is not a good design. If those are meant only for the internal uses of Driver class, you could keep them as private properties of Driver class.

#include <vector>
#include <map>
#include <string>
#include <iostream>


class Driver /* final */ // -> optional
{
private: // Student is private for Driver class
    class Student
    {
        // type alias is enough for the map
        using ReportCard  = std::map<std::string, double>;
    private:
        int m_roll_no;
        std::string m_name;
        ReportCard m_reportCard;

    public:
        Student(int inRollNo, const std::string& inName)
            : m_roll_no{ inRollNo }, m_name{ inName }
        {}
        // make the member functions const if they are not modifing the members
        const std::string& getName() const { return m_name; }
        int getRollNo() const { return m_roll_no; }
        ReportCard& getReportCard() { return m_reportCard; }
        std::size_t getReportCardSize() const { return m_reportCard.size(); }
    };

private:
    std::vector<Student> student_list;
    std::vector<Student> temp;

public:
    void studentTestPopulate()
    {
        // construct the `Student` in-place using `std::vector::emplace_back`
        student_list.emplace_back(1, "Tim"); 
        student_list.emplace_back(2, "Matt");
        student_list.emplace_back(100, "Luke");
        student_list.emplace_back(68, "Lissy");
        student_list.emplace_back(20, "Tony");
        student_list.emplace_back(33, "Joseph");
        student_list.emplace_back(14, "Sid");
        student_list.emplace_back(15, "Roby");
        student_list.emplace_back(44, "Rohan");
        student_list.emplace_back(11, "Kevin");
        student_list.emplace_back(19, "George");
    }

    void reportCardPopulate()
    {
        for (auto& student : student_list)
        {
            std::cout << student.getName() << "\n";

            student.getReportCard().emplace(student.getName(), 12.0);
            //                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
            // use `std::map::emplace` for constructing `ReportCard` in-place
            for (auto& mark : student.getReportCard())
            {
                std::cout << mark.first << " " << mark.second << "\n";
            }
        }
    }
    // ... other members
};

int main()
{
    Driver driver;
    driver.studentTestPopulate();
    driver.reportCardPopulate();
}
JeJo
  • 30,635
  • 6
  • 49
  • 88
  • Thank you for the detailed explanation. :) – james Aug 29 '19 at 10:15
  • How do we know which functions to declare as const? getName() returns something yet it is const however getReportCard also returns something but it isn't const. Also why do we write const before the function declaration sometimes (const std::string& getName() const { return m_name; } here const comes first) and after other times (int getRollNo() const { return m_roll_no; } here const comes later)? – james Aug 29 '19 at 10:50
  • @james a const function declaration is like `return_type funcName() const {...}`. See the `const` which makes the function `const` meaning it does not change the members of the class inside the definition of the function. Secondly, `const std::string& getName() const { return m_name; } ` here the function is `const` and we return the `const` qualified reference to the member `m_name`, meaning the member is niether is modified inside the function nor can be modified outside, rather the caller can only read it. – JeJo Aug 29 '19 at 10:59
  • @james Read more about the const functions: [What is meant with “const” at end of function declaration?](https://stackoverflow.com/questions/3141087/what-is-meant-with-const-at-end-of-function-declaration) – JeJo Aug 29 '19 at 11:00