patterncppMinor
OOP elevator design evaluation
Viewed 0 times
elevatorevaluationoopdesign
Problem
I am not so much used to OOP designing, So I took one of the very commonly used interview question , "Designing the Elevator System". Below is the prototype.
I would like to get some feedback on the design like
I know that these kind of things have no finite answer or just one answer but I would like to know my required areas of improvement.
Original Design:
```
class elevator{
private:
//The Lift box the elevator controls
liftbox box;
//The total number of levels
const int LEVEL;
//The request for various floors
set req;
//Triggers the search to move to the next floor if required
void moveLiftToNext();
//Instructs the lift box to move to that floor
void moveLiftTo(int);
public:
//Adds request to the queue
void addRequest(int);
//Gets the total levels the box can move
int getLevel();
//For Emergency
void setEmergency();
void unsetEmergency();
};
class liftbox{
private:
dailpad pad;
elevator ele;
int currLevel; //Current position
bool direction; //True - up ; False- Down
public:
//Instruction to move the box to certain level
void Move(int x){
if(x = ele.getLevel())
return; //invalid number
//Move the lift
//update direction and currLevel
}
//returns the current level. Used by Elevator
int getCurrLevel();
//returns the current direction of movement.Used by Elevator
int getDirection();
void setEmergency(){
//Do the required
ele.setEmergency();
}
void unsetEmergency(){
ele.unsetEmergency();
}
//can passed to the elevator
void addRequest(int);
};
class dailpad{
private:
//Some DS to represent the dailpad
//Lift box is belongs to
liftbox box;
public:
void recieveComm
I would like to get some feedback on the design like
- Best practices which will have a particular aspect
- Flexibility in the code
- Some important generic feature missing
- Areas to concentrate to write better code and design
I know that these kind of things have no finite answer or just one answer but I would like to know my required areas of improvement.
Original Design:
```
class elevator{
private:
//The Lift box the elevator controls
liftbox box;
//The total number of levels
const int LEVEL;
//The request for various floors
set req;
//Triggers the search to move to the next floor if required
void moveLiftToNext();
//Instructs the lift box to move to that floor
void moveLiftTo(int);
public:
//Adds request to the queue
void addRequest(int);
//Gets the total levels the box can move
int getLevel();
//For Emergency
void setEmergency();
void unsetEmergency();
};
class liftbox{
private:
dailpad pad;
elevator ele;
int currLevel; //Current position
bool direction; //True - up ; False- Down
public:
//Instruction to move the box to certain level
void Move(int x){
if(x = ele.getLevel())
return; //invalid number
//Move the lift
//update direction and currLevel
}
//returns the current level. Used by Elevator
int getCurrLevel();
//returns the current direction of movement.Used by Elevator
int getDirection();
void setEmergency(){
//Do the required
ele.setEmergency();
}
void unsetEmergency(){
ele.unsetEmergency();
}
//can passed to the elevator
void addRequest(int);
};
class dailpad{
private:
//Some DS to represent the dailpad
//Lift box is belongs to
liftbox box;
public:
void recieveComm
Solution
First the C++ code:
This is not going to work as you expect it.
Each class contains an object of the other type means that both will have completely different objects. If you want them to refer back to each other in some sort of parent child relationship then one of them needs to be a reference or pointer to the parent.
Traditional all caps is reserved for macros. Break the tradition at your own cost.
Methods that return information about the state of the object without actually changing the state should be marked const.
I am not sure I agree with some of the decisions about making methods public. I think very few objects should get to interact with these systems sometimes friendship can help in limitting access. (Now friendship increases coupling with the friend, but if it decreases the external public interface it will decrease coupling with objects that have no rights to modify the object). Anyway I would expect to see a justification as to why methods are public. If anybody should be able to call them fine. If nobody but another object should call them you need to make a better case.
Design
Looking at it from a design perspective.
Not sure I see the distinction between an elevator/liftbox
In big buildings some lifts do not go to all floors.
One of the things about elevators is that they usually come in banks and do not operate independently. For really big building multiple banks will be combined but will work independently (unless there is some major emergency). How are you going to organize your code so that multiple lifts(sorry elevators) can work together. Also I want to see how you can decouple the elevator object from bank control logic. (ie I don't expect to see all the control logic in the bank, I would like to see control logic in the elevator but decision logic in bank). But if their are multiple banks to coordinate I want the logic for a higher level control.
The real meat of this problem is how to de-couple the objects from each other. What patterns do you think are being used here.
Updated Code:
Not sure you understand a bank of elevators work (based on the code).
A bank is a group of 2 or more elevators that work together to serve a set of floors.
I still do not see the pattern you are using to decouple the elevators from the Bank.
class elevator{
liftbox box;
class liftbox{
elevator ele;This is not going to work as you expect it.
Each class contains an object of the other type means that both will have completely different objects. If you want them to refer back to each other in some sort of parent child relationship then one of them needs to be a reference or pointer to the parent.
const int LEVEL;Traditional all caps is reserved for macros. Break the tradition at your own cost.
int getCurrLevel();
int getDirection();Methods that return information about the state of the object without actually changing the state should be marked const.
public/privateI am not sure I agree with some of the decisions about making methods public. I think very few objects should get to interact with these systems sometimes friendship can help in limitting access. (Now friendship increases coupling with the friend, but if it decreases the external public interface it will decrease coupling with objects that have no rights to modify the object). Anyway I would expect to see a justification as to why methods are public. If anybody should be able to call them fine. If nobody but another object should call them you need to make a better case.
Design
Looking at it from a design perspective.
Not sure I see the distinction between an elevator/liftbox
In big buildings some lifts do not go to all floors.
One of the things about elevators is that they usually come in banks and do not operate independently. For really big building multiple banks will be combined but will work independently (unless there is some major emergency). How are you going to organize your code so that multiple lifts(sorry elevators) can work together. Also I want to see how you can decouple the elevator object from bank control logic. (ie I don't expect to see all the control logic in the bank, I would like to see control logic in the elevator but decision logic in bank). But if their are multiple banks to coordinate I want the logic for a higher level control.
The real meat of this problem is how to de-couple the objects from each other. What patterns do you think are being used here.
Updated Code:
Not sure you understand a bank of elevators work (based on the code).
A bank is a group of 2 or more elevators that work together to serve a set of floors.
I still do not see the pattern you are using to decouple the elevators from the Bank.
Code Snippets
class elevator{
liftbox box;
class liftbox{
elevator ele;const int LEVEL;int getCurrLevel();
int getDirection();public/privateContext
StackExchange Code Review Q#5723, answer score: 5
Revisions (0)
No revisions yet.