I am having a hard time achieving the kind of data access I want in C++ classes

Question

Basically, I have a class "A" with various private data. Class "B" is a singly linked list of pointers unto "A" objects. Class "C" has other (non-essential for the purposes of my query) data, and an object "B", in other words one list of A's. I'm supposed to do a simple kind of C++ project for school with this kind of class nesting. I guess I could "fix" everything by making all data public, but that's atrocious.

drug.h

#ifndef DRUG_H
#define DRUG_H
#include "clientlist.h"

// forward declaration
class ClientList;

class Drug {

    private:

    ClientList clients;

    std::string name;
    std::string description;
    double price;
    int stock;
    int tobuy;

    public:

    Drug(std::string n = "drug", std::string d = "description", double p = 0.0, int s = 0, int b = 0);

    void print();
};

#endif

drug.cpp

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

Drug::Drug(string n, string d, double p, int s, int b) {
    name = n;
    description = d;
    price = p;
    stock = s;
    tobuy = b;
}

void Drug::print() {
    cout.setf(ios::fixed);
    cout.setf(ios::showpoint);
    cout << setprecision(2);
    cout << "price is: " << price << " PLN" << endl;
    cout << "stock is: " << stock << endl;
    cout << "tobuy is: " << tobuy << endl;
    cout << "name is: " << name << endl;
    cout << "description is: " << description << endl;
}

clientlist.h

#ifndef CLIENT_LIST_H
#define CLIENT_LIST_H

//forward declaration
class Client;

class ClientList {

    private:

    struct clientlistelement {
    Client *data;
    clientlistelement *next;
    };

    clientlistelement *head;

    int numberofclients;

    public:

    ClientList();
    void print();
};

#endif

main.cpp

#include <iostream>
#include <string>
#include "drug.h"
#include "druglist.h"
#include "client.h"
#include "clientlist.h"
using namespace std;

int main() {

    Drug d;
    Client c;

    d.print();
    cout << d.clients.numberofclients << endl; // <-- error

    cout << endl;

    c.print();

    return 0;
}

Maybe my design is just shitty, but basically a drug is designed to contain a "list" object, which is essentially just a list of pointers unto client objects, which in turn have lists of drugs. The whole point of this two-way mirrored interaction is to have singly linked lists of two types of objects which are mutually "assigned" (read: pointing on) each other, to simulate services which are aware of their customers and vice versa. But I've very quickly stumbled upon this issue of not being able to access hardly any data. I tried friending my print members to no avail and I'm not sure where to proceed. Can anybody lend some ideas?

EDIT: I am by no means an experienced or good C++ programmer, this is only the beginning of my first class. I guess what I'm after, that I don't understand, is somehow being to absorb access to private data in an object of a different class. =/

Answer 1

I don't necessarily see a problem with the model of Drug having ClientList having Clients, other than the class names might not accurately reflect what these are used for. The "Drug" seems not so much the drug itself, as a "DrugAvailability" -- how much is in stock, how much is needed to buy. The Clients could be "PrescriptionForDrug", which would probably include a count of how many are needed.

Then in object terms, I would not make the client list a public member of DrugAvailability. It should be kept private because nobody should care about what is listed inside of DrugAvailability. You just want to know, how many are needed and how many are in stock. You want to hide the "how it's done" as much as possible.

For example, the "tobuy" does not need to be an int member. Instead, it can be a member function, ToBuy(), and instead of tracking an int member, when this function is called it can (privately) run down the list of PrescriptionForDrug (i.e. the client list) and count up how many are needed, and compare that to stock.

So in that case, DrugAvailability.Stock() would return how many are in stock, DrugAvailability.ToBuy() would return how many are needed, and it would account for any prescriptions. You don't really need to know that there are PrescriptionForDrug listed inside of DrugAvailability, so you keep that detail private.

It's important to come up with the right names of things, to find out exactly what you are trying to solve. Then, hide the details and only surface the parts that the outside world really needs to know about.

Answer 2

I would suggest breaking apart your basic design. Classes in C++ (and in Object-Oriented design in general) should model some real-world object. To that end, a Drug can have attributes (name, price, etc.), but Client (people that take the Drug) are not attributes of a Drug and should not be included in that class definition.

In that same vein, a Client is presumably a person, which again can have specific attributes (name, age, gender, etc.). Any Drug that a Client may or may not be taking is not really an attribute of that Client.

I would model this so that Drug and Client are two separate distinct classes, with no attributes of one held within the other. Once that is done, you can use separate classes to model the relationships between Drug and Client.

For instance:

A Prescription object can associate one Client and multiple Drug objects, while another class can handle the association in reverse (one Drug and all the associated Client objects).

Being as how this is homework, I don't want to actually give you the code to do this, but I wanted to get you thinking in the right "object-oriented" way.

If as you say, this basic design is forced upon you (a very unfortunate situation, as the design itself is not ideal, the data is too highly coupled and really goes against some decent object-oriented principles). You can simply add accessor (getter) functions for your data types. This is different than making the data public, in that you cannot actually change the data, but you can get it for purposes of displaying output, etc.