1

Consider the below C++ code

class B;

class A{
private:
    B* mB;
};

class B{

private:
    doSomethingImportant();

};

We have a Object A that contains (has a) Object B. The parent being A and child being B. Now if I want A to make B do doSomethingImportant() , I see that adding A as a friend of B is the only way to do it.

friend class A inside class B. This would enable A's functions to access B's private function.

I find this approach a little weird since creates a loophole in the Data_Hiding concept. Is there a better way to establish a parent-child relationship between the object ? or is this the best way ?


Adding my actual motivation for this question

class elevator{
private:
    //The Lift box the elevator controls
    liftboxControlUnit & mLiftBoxCtrlUnit;   

    //constructor
    elevator(int Level=1, int NoOfBanks =1 );

    //Destructor
    ~elevator();

    //Triggers the search to move to the next floor if required 
    void moveLiftToNext();

public:

    //Adds request to the queue
    void addRequest(int FloorNumber){

    //Add the request to the queue. The single button outside the elevator door
    mLiftBoxCtrlUnit.addRequest(FloorNumber);

    }

    //For Emergency. Should be accessible to everyone !
    void setEmergency();
    void unsetEmergency();

};

typedef enum Direction{
    UP,
    DOWN
}direction;

class liftboxControlUnit{
private:

    //The request for various floors
    set<int> mRequestQueue;

    //The various banks for the whole system
    vector<Bank> mBanks;

    //The total number of levels. Remains the same for one building
    const int mTotalLevel;

    //Instruction to move the box to certain level
    void processRequest(){

        //Do the logic to move the box.

    }

    //can passed to the elevator
    void addRequest(int x){
        mRequestQueue.insert(x);
    }

    //Can be set by elevator class
    void setEmergency(){
        //Do the required 
        //Set Emergency on all Banks
    }

    void unsetEmergency(){
        //UnsetEmegency on all banks
    }

    void emergencyListener(){
        //Listen to all the banks if emergency has been set
    }

    void BankFreeListener(){
        //Listen to the banks if any is free

        //If so then
        processRequest();
    }

public:
    //Constructor
    liftboxControlUnit(int TotalLevels, int NoOfBanks): mTotalLevel(TotalLevels){
        for(int i=0 ; i lessthan NoOfBanks; ++ i)
            mBanks.push_back(Bank(0,UP));
    }    
    friend class elevator;
};

class Bank{
private:

    //The dailpad inside the bank
    dailpad & mpad;

    //Current Location
    int mPresentLevel;

    //Current direction of movement
    direction mDirection;

    //Currently moving
    bool mEngaged;

    //Manipulate the bank
    void move(int NoOfMoves){
        setEngaged();

        //Move the elevator

        unsetEngaged();    
    }

    //getters
    int getPresentLevel() const;
    int getDirection() const;

    //setters
    void setPresentLevel(int);
    void setDirection(direction);

    //Manipulate the engaged flag
    bool isEngaged() const;
    bool setEngaged();
    bool unsetEngaged();

    //For emergency
    void reset();

    //Dailpad Listener
    void dailpadListener(){

    }


public:
    Bank(int StartingLevel, direction Direction): mPresentLevel(StartingLevel),
            mDirection(Direction),
            mEngaged(false),
            mpad()
    {

    }

    //For emergency . Should be available for all.
    void SetEmergency();
    void UnsetEmergency();
    bool isEmergency();

    friend class liftboxControlUnit;
};


class dailpad{

private:
    //Some DS to represent the state . probably a 2D Array.

    void renderDisplay();

public:

    //Constructor
    dailpad();

    void getCommand(int x){
        //Depending on the value we can do the following

        //Make necessary changes to the display
        renderDisplay();
    }

    friend class Bank;
};
Keith Thompson
  • 254,901
  • 44
  • 429
  • 631
bsoundra
  • 930
  • 2
  • 13
  • 27
  • This code wouldn't compile. `B` object cannot be declared until `class B` body is visible. You can declare only reference / pointer. – iammilind Nov 02 '11 at 03:19
  • @iammilind : Yes, you are right. – bsoundra Nov 02 '11 at 03:21
  • 2
    I think it would help to give at least some sort of concrete idea of what you're really trying to accomplish here. The bottom line is that you can either give A access to B::doSomethingImportant or not, and it's up to you to decide whether you *want* to give it that access. – Jerry Coffin Nov 02 '11 at 03:30
  • I was trying to implement a OOP design for elevator problem . http://codereview.stackexchange.com/questions/5723/c-code-for-oop-elevator-design-evaluation . Here the parent (liftboxControlUnit) needs to instruct the child(bank - one lift box) to move up and down and none other than parent should be able to do it. So I had to make the _liftboxControlUnit_ a friend of _bank_ . Thats the reason behind this question. – bsoundra Nov 02 '11 at 03:38
  • I completely agree with Jerry. One of the main reason I see doSomethingImportant() to be a private is that it must be implementing som very specific functionality and would be used inside other public member functions, So that would mean to reach doSomethingImportant func it would take atleast 2 function calls. – Arunmu Nov 02 '11 at 03:40
  • I have updated the question with actual motivation – bsoundra Nov 02 '11 at 03:52

2 Answers2

1

IMO, for this task you should probably nest the "lift box" class inside of the controller class:

class lift_controller { 

    class lift_box { 
        open_doors();
        close_doors();
        move_to_floor();
    };

    std::vector<lift_box> bank;
};

To the outside world, there need be no evidence that lift_box exists at all. It communicates exclusively with the lift_controller, and all outside communication with a lift_box goes through the lift_controller.

In this case (only lift_controller has access to lift_box at all), it seems clear (at least to me) that any operations the lift_controller may need to invoke on a lift_box should just be made public functions of lift_box. To enforce nobody else having access to lift_box, ensure that the definition of lift_box is in the private: section of lift_controller.

Edit: I should add that quite a bit of the design you've edited into your question above makes little or no sense to me. Just for example, you have things like direction and present level for the bank. Unless I'm completely misunderstanding what you mean by a bank, this seems like a clear error to me -- the bank isn't at a particular level or moving in a particular direction. Rather, each individual elevator in the bank is at some level and (potentially) moving in some direction.

Jerry Coffin
  • 476,176
  • 80
  • 629
  • 1,111
  • Thats a very nice observation and your suggestion holds good for this design. Suppose there is an inventory control module, that has reference to the lifts (different banks may be controlled by different inventory unit ), then lift_box has to come out. The main thing,What I am expecting is not a work-around but a generic design solution (if there is any) – bsoundra Nov 02 '11 at 04:04
  • I wrote the code, considering that Bank - a single lift box . sorry, for the confusing terminology – bsoundra Nov 02 '11 at 04:08
  • @bsoundra: I think to come up with a generic design solution, you have to come up with a generic design problem. There may be a generic problem there, but if so I don't think you've really expressed what it is. I don't think what I've given above is a workaround, but simply a better design. As far as your example (inventory control on a removed lift box) goes, I'd say a lift box that's been removed from the rest of the elevator mechanism no longer implements the same class at all. After removal, it's not a lift box, just a hunk of metal that could, perhaps, become a lift box again some day. – Jerry Coffin Nov 02 '11 at 04:10
  • @bsoundra: okay -- I definitely think that needs some cleanup. I, at least, would think of "bank" (relative to elevators) as meaning all the elevators that might (for example) show up when I press a particular up or down button. – Jerry Coffin Nov 02 '11 at 04:12
  • A generic problem would be where an object, say B, is child of A and there exist another object C which has a reference to the object B, as a member variable. Now, if a set of operations allowed on B by A should not be available to C , how do we do it ? It so seems that I am creating a problem just to deal with this question. Would there be any scenario like this ? If there is, would it be considered a bad design ? – bsoundra Nov 02 '11 at 04:21
  • As I have described in my answer, if you require that level of access customization, Object C should hold a different interface to object B instead of B itself. B can implement multiple abstract super classes as interfaces. When Object B is accessed through these interfaces, the unnecessary public functions will be hidden. – devil Nov 02 '11 at 04:37
  • @bsoundra: As devil said, multiple inheritance can be a way to deal with this. Generally, however, an object should represent some single thing, so it's frequently a design problem when you want two different things to see it completely differently. – Jerry Coffin Nov 02 '11 at 05:43
0

You seem to want class A to only be able to access one private function in B, B::doSomethingImportant() and no other private functions.

This usually means that B::doSomethingImportant() should really be public. Like this, A will not be able to access other private data members of B.

Further, if you do not want other classes to access B::doSomethingImportant(), they should not hold a pointer to B but instead, a hold a pointer to an interface (abstract super class) of B that does not expose B::doSomethingImportant().

Or perhaps other classes only read data from B. In that case they can hold B const * which will not allow them to call B::doSomethingImportant() unless they do a const_cast.

devil
  • 1,829
  • 16
  • 23