HiveBrain v1.2.0
Get Started
← Back to all entries
principlecppMinor

Selecting two shapes to compare their areas and volumes

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
volumestwoshapesselectingandareascomparetheir

Problem

How can I refactor these switch cases?

```
void selectOption(Shape** shapes, int size, int selection) {
int obj1;
int obj2;
bool init2 = false;
bool init = false;
int option = 0;

do {
cout > obj1;
switch (obj1) {

case 1:
do {
cout > obj2;

switch (obj2) {
case 1:
if (selection == 1) {
compareArea(&shapes[0], &shapes[0]);
}
else if (selection == 2) {
compareVolume(&shapes[0], &shapes[0]);
}
else {
cout getID() print();
cout getID() print();
cout getID() print();
cout getID() print();
}
init2 = true;
break;
case 3:
if (selection == 1) {
compareArea(&shapes[0], &shapes[2]);
}
else if (selection == 2) {
compareVolume(&shapes[0], &shapes[2]);
}
else {
cout getID() print();
cout getID() print();
}
init2 = true;
break;
case 4:
if (selection == 1) {
compareArea(&shapes[0], &shapes[3]);
}
else if (selection == 2) {
compareVolume(&shapes[0], &shapes[3]);
}
else {
cout getID() print();
cout getID() print();
}
init2 = true;
break;
case 5:
if (selection == 1) {
compareArea(&shape

Solution

If you hard-code less data, you can easily compress these:

case 1:
    if (selection == 1) {
        compareArea(&shapes[0], &shapes[0]);
    }
    else if (selection == 2) {
        compareVolume(&shapes[0], &shapes[0]);
    }
    else {
        cout getID() print();
        cout getID() print();
        cout getID() print();
        cout getID() print();
    }
    init2 = true;
    break;


Here, you are switching over obj2, so you can do something like this:

case 1:
case 2:
    if (selection == 1) {
        compareArea(&shapes[0], &shapes[obj1 - 1]);
    }
    else if (selection == 2) {
        compareVolume(&shapes[0], &shapes[obj1 - 1]);
    }
    else {
        cout getID() print();
        cout getID() print();
        cout << "\n";
    }
    init2 = true;
    break;


If you replace the hard-coded 0's in the selection above with obj1 - 1, you should be able to do away with the switches entirely.

I believe you have a bug in case 8 as it does not match the pattern:

else {
    cout getID() print();
    cout getID() print();
}

Code Snippets

case 1:
    if (selection == 1) {
        compareArea(&shapes[0], &shapes[0]);
    }
    else if (selection == 2) {
        compareVolume(&shapes[0], &shapes[0]);
    }
    else {
        cout << "\n  " << shapes[0]->getID() << " --";
        shapes[0]->print();
        cout << "\n  " << shapes[0]->getID() << " --";
        shapes[0]->print();
        cout << "\n";
    }
    init2 = true;
    break;
case 2:
    if (selection == 1) {
        compareArea(&shapes[0], &shapes[1]);
    }
    else if (selection == 2) {
        compareVolume(&shapes[0], &shapes[1]);
    }
    else {
        cout << "\n  " << shapes[0]->getID() << " --";
        shapes[0]->print();
        cout << "\n\n  " << shapes[1]->getID() << " --";
        shapes[1]->print();
    }
    init2 = true;
    break;
case 1:
case 2:
    if (selection == 1) {
        compareArea(&shapes[0], &shapes[obj1 - 1]);
    }
    else if (selection == 2) {
        compareVolume(&shapes[0], &shapes[obj1 - 1]);
    }
    else {
        cout << "\n  " << shapes[0]->getID() << " --";
        shapes[0]->print();
        cout << "\n  " << shapes[obj1 - 1]->getID() << " --";
        shapes[obj1 - 1]->print();
        cout << "\n";
    }
    init2 = true;
    break;
else {
    cout << shapes[0]->getID() << " --";
    shapes[0]->print();
    cout << shapes[6]->getID() << " --";
    shapes[7]->print();
}

Context

StackExchange Code Review Q#113704, answer score: 6

Revisions (0)

No revisions yet.