1

Context

My professor gave me a task to make a program using aggregation between 2 classes while also separating the classes into a .h and .cpp files.

My solution

The header file containing the class declaration:

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

class medicalCompany {
private:
    string ceoName;
    string email;
    string phoneNumber;
    string locate;
public:
    medicalCompany();
    void Name(string n);
    void mail(string m);
    void phone(string p);
    void location(string l);
    ~medicalCompany();

};
class origin {
private:
    medicalCompany country;
    
public:
    origin();
    void address();
    ~origin();

};

and my .cpp file:

#include <iostream>
#include "function.h"
#include <string>
using namespace std;
medicalCompany::medicalCompany() {
    cout << "OUR COMPANY IS GLAD TO BE OF SERVICE !" << endl;
    cout << "****************************************************" << endl;
}
void medicalCompany::Name(string n){
    ceoName = n;
    cout << "OUR CEO IS " << endl;
    cout<< ceoName << endl;
    cout << "****************************************************" << endl;
}
void medicalCompany::mail(string m) {
    email = m;
    cout << "USE OUR EMAIL TO CONTACT US : " << endl;
    cout<< email << endl;
    cout << "****************************************************" << endl;
}
void medicalCompany::phone(string p) {
    phoneNumber = p;
    cout << "THIS IS OUR CUSTOMER SERVICE PHONE NUMBER " << endl;
    cout<< phoneNumber << endl;
    cout << "****************************************************" << endl;
}
void medicalCompany::location(string l) {
    locate = l;
    cout << " OUR COMPANY IS LOCATED IN " << endl;
    cout << locate << endl;
    cout << "****************************************************" << endl;
}
medicalCompany::~medicalCompany() {
    cout << "thanks for trusting our company ^_^" << endl;
    cout << "****************************************************" << endl;
}
origin::origin() {
    cout<< "constructor 2"<<endl;
}
void origin::address() {
    cout << country.location;
}
origin::~origin() {
    cout << "bye" << endl;
}

The two classes are used in my main.cpp file:

#include <iostream>
#include <string>
#include "function.h"
using namespace std;

int main() {

    medicalCompany o;
    o.Name("jack");
    o.mail("ouremail@company.com");
    o.phone("2342352134");
    o.location("Germany");
    origin o2;

    return 0;
}

Problem

I run into this error :

Severity Code Description Project File Line Suppression State
Error   C3867 'medicalCompany::location': non-standard syntax; use '&' to create a pointer to member    CP2_HW  c:\function.cpp 41 
WaterFox
  • 850
  • 6
  • 18
nabil497
  • 113
  • 1
  • 13
  • 2
    `cout << country.location;` location is declared as `void location(string l);` so its completely incompatible with outputting to `cout` even if you called it correctly as a function . Did you mean `cout << contry.locate;` – drescherjm Dec 06 '20 at 18:24
  • i want to call the location function using an object from medicalCompany class inside the origin class and i don't know exactly how :( – nabil497 Dec 06 '20 at 18:29
  • 2
    I am pretty sure the error message was about what I said. This line here: `cout << country.location;` – drescherjm Dec 06 '20 at 18:30
  • on a side note, `using namespace std;` is usually considered bad practice, and should be avoided, as the cost of possible risks does not overweight the benefit of not having to type `std::cout`. Check for example [here](https://www.geeksforgeeks.org/using-namespace-std-considered-bad-practice/) for a more detailed explanation. – WaterFox Dec 06 '20 at 18:31
  • when i try to make it country.locate; instead of country.location; i get this locate is inaccessible – nabil497 Dec 06 '20 at 18:32
  • 2
    yes because locate is a private member. You can either replace `void origin::address(){cout << country.location;}` by `void origin::address(){country.location();} or by `void origin::address(){cout << country.locate;} if you put the locate member as a public variable. – WaterFox Dec 06 '20 at 18:35
  • @drescherjm please give instructions to make the code work :( i guess i didn't get what you are telling me :( – nabil497 Dec 06 '20 at 18:36
  • 1
    Make `locate` a public member. Or add a public function in `medicalCompany` to return it. – drescherjm Dec 06 '20 at 18:36
  • @ArnaudBecheler thank you for you help and advice ^_^ , please guys can you give me instructions to make the code work ? :( i didn't get the idea – nabil497 Dec 06 '20 at 18:38
  • @drescherjm ok i will try and see if it works with me thank you ^_^ – nabil497 Dec 06 '20 at 18:40
  • 1
    @ArnaudBecheler the solution worked thanks a lot i was really frustrated here didn't know that to do XD you are a life saver sir thank you ^_^ – nabil497 Dec 06 '20 at 18:44
  • 2
    @drescherjm the problem was fixed by the line mr. arnaud wrote and the problem was exactly as you said drescherjm so thank you so much guys for the help ^_^ – nabil497 Dec 06 '20 at 18:46
  • @drescherjm welcome ! You can upvote drescherjm comments, as he was helping too with the correct hints :) – WaterFox Dec 06 '20 at 19:04
  • @ArnaudBecheler i did of course ^_^ thank you both for the amazing help – nabil497 Dec 07 '20 at 10:38

1 Answers1

1

You can either:

  • replace void origin::address(){cout << country.location;} by void origin::address(){country.location();}

  • or by void origin::address(){cout << country.locate;} if you put the locate member as a public variable.

Also, few remarks:

  • Generally you would prefer avoiding exposing private members, so the first solution should be prefered.

  • the instruction using namespace std; is usually considered bad practice, and should be avoided, as the cost of possible risks does not overweight the benefit of not having to type std::cout(see this question for more information)

  • In terms of naming convention, I would have exchanged the names of locate and location: location should be the member variable and locate the action (function) of getting the location.

  • Prefer using a constructor and intialization lists rather than getter/setter.

  • Your output formatting should be very separate from the logic of your classes, for example implementing a << operator for your class.

Following this logic, your code should look more like:

#include <iostream>
#include <string>

class medicalCompany {
private:
  std::string _ceoName;
  std::string _email;
  std::string _phoneNumber;
  std::string _location;
public:
  
  // Constructor
  medicalCompany(std::string name, std::string email, std::string phone, std::string location):
  _ceoName(name), 
  _email(email), 
  _phoneNumber(phone), 
  _location(location)
  {}
  friend ostream& operator<<(ostream& os, const medicalCompany& dt);
  };

and for the ostream operator:

ostream& operator<<(ostream& os, const medicalCompany& co)
{
    os << co._ceoName << " " co._phoneNumber << ' ' << co._email;
    return os;
}

This would allows to write code like this in your main:

int main() {
  medicalCompany o("jack", "ouremail@company.com","2342352134","Germany")
  std::cout << o << std::endl;   
  return 0;
}

The code is not functional and you would have to edit it to fit your formating requirement, but you have the idea :) Good luck!

WaterFox
  • 850
  • 6
  • 18
  • 1
    it worked sir thank you for your help and great advice , and it is better to exchange the names thank you – nabil497 Dec 06 '20 at 18:48
  • 1
    you're welcome ! C++ can be confusing sometimes, especially when you begin and @drescherjm had the solution too ! – WaterFox Dec 06 '20 at 18:57