Skip to main content
Code Review

Return to Answer

There is a lot to be improved here. Number one is that you have huge blocks of code in super methods that handle entire moves. You should split that up into smaller logical groups for ease of reading, debugging, maintenance, and additions. One example I noticed is that you have this a lot:

cout << "\n 1. Attack the thief" << endl;
cout << "\n 2. Attempt to flee" << endl;

Maybe you should make a method to print the options:

void printOptions(std::vector<std::string> ops)
{
 for(int i =const 0;auto& iopt <: ops.size(); i++) {
 std::cout << i + 1 <<&elem ".- "&v[0] << ops[i]opt << std::endl;
 }
}

Then, you can just call this:

printOptions({"Attack the thief", "Attempt to flee"});

This is cleaner than the other version, and is reusable, so it will shorten your code. You can also have as many arguments as you wish, so if you want three options sometime, no problem.

Another problem you have is you are using namespace std;. This is bad, because later you may define your own namespace or use a different one that has some methods named the same, which could cause problems.

Your switch statements should not look like this:

switch (input) {
 case 1:
 searchBody();
 case 2:
 riverstead();
}

Instead, the case statements should be indented, like this:

switch (input) {
 case 1:
 searchBody();
 case 2:
 riverstead();
}

Also, I believe you have an error here. In C++, and most other languages, once a matching case statement is reached, you continue executing all lower case statements. If case 1: is matched and searchBody(); is executed, that means riverstead(); is also executed. You should add a break; statement to the end of each case block:

switch (input) {
 case 1:
 searchBody();
 break;
 case 2:
 riverstead();
 break; // unnecessary here because it is the last statement, but good practice.
}

I prefer if/else statements to switch statements, but switch statements are sometimes helpful.

Again, you have huge methods that should be split up into smaller blocks. I'm sure there are other things you can clean up, but this will be a good startmstart

There is a lot to be improved here. Number one is that you have huge blocks of code in super methods that handle entire moves. You should split that up into smaller logical groups for ease of reading, debugging, maintenance, and additions. One example I noticed is that you have this a lot:

cout << "\n 1. Attack the thief" << endl;
cout << "\n 2. Attempt to flee" << endl;

Maybe you should make a method to print the options:

void printOptions(std::vector<std::string> ops)
{
 for(int i = 0; i < ops.size(); i++) {
 std::cout << i + 1 << ". " << ops[i] << std::endl;
 }
}

Then, you can just call this:

printOptions({"Attack the thief", "Attempt to flee"});

This is cleaner than the other version, and is reusable, so it will shorten your code. You can also have as many arguments as you wish, so if you want three options sometime, no problem.

Another problem you have is you are using namespace std;. This is bad, because later you may define your own namespace or use a different one that has some methods named the same, which could cause problems.

Your switch statements should not look like this:

switch (input) {
 case 1:
 searchBody();
 case 2:
 riverstead();
}

Instead, the case statements should be indented, like this:

switch (input) {
 case 1:
 searchBody();
 case 2:
 riverstead();
}

Also, I believe you have an error here. In C++, and most other languages, once a matching case statement is reached, you continue executing all lower case statements. If case 1: is matched and searchBody(); is executed, that means riverstead(); is also executed. You should add a break; statement to the end of each case block:

switch (input) {
 case 1:
 searchBody();
 break;
 case 2:
 riverstead();
 break; // unnecessary here because it is the last statement, but good practice.
}

I prefer if/else statements to switch statements, but switch statements are sometimes helpful.

Again, you have huge methods that should be split up into smaller blocks. I'm sure there are other things you can clean up, but this will be a good startm

There is a lot to be improved here. Number one is that you have huge blocks of code in super methods that handle entire moves. You should split that up into smaller logical groups for ease of reading, debugging, maintenance, and additions. One example I noticed is that you have this a lot:

cout << "\n 1. Attack the thief" << endl;
cout << "\n 2. Attempt to flee" << endl;

Maybe you should make a method to print the options:

void printOptions(std::vector<std::string> ops)
{
 for(const auto& opt : ops) {
 std::cout << &elem - &v[0] << opt << std::endl;
 }
}

Then, you can just call this:

printOptions({"Attack the thief", "Attempt to flee"});

This is cleaner than the other version, and is reusable, so it will shorten your code. You can also have as many arguments as you wish, so if you want three options sometime, no problem.

Another problem you have is you are using namespace std;. This is bad, because later you may define your own namespace or use a different one that has some methods named the same, which could cause problems.

Your switch statements should not look like this:

switch (input) {
 case 1:
 searchBody();
 case 2:
 riverstead();
}

Instead, the case statements should be indented, like this:

switch (input) {
 case 1:
 searchBody();
 case 2:
 riverstead();
}

Also, I believe you have an error here. In C++, and most other languages, once a matching case statement is reached, you continue executing all lower case statements. If case 1: is matched and searchBody(); is executed, that means riverstead(); is also executed. You should add a break; statement to the end of each case block:

switch (input) {
 case 1:
 searchBody();
 break;
 case 2:
 riverstead();
 break; // unnecessary here because it is the last statement, but good practice.
}

I prefer if/else statements to switch statements, but switch statements are sometimes helpful.

Again, you have huge methods that should be split up into smaller blocks. I'm sure there are other things you can clean up, but this will be a good start

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

There is a lot to be improved here. Number one is that you have huge blocks of code in super methods that handle entire moves. You should split that up into smaller logical groups for ease of reading, debugging, maintenance, and additions. One example I noticed is that you have this a lot:

cout << "\n 1. Attack the thief" << endl;
cout << "\n 2. Attempt to flee" << endl;

Maybe you should make a method to print the options:

void printOptions(std::vector<std::string> ops)
{
 for (int i = 0; i < ops.size(); i++) {
 std::cout << i + 1 << ". " << ops[i] << std::endl;
 }
}

Then, you can just call this:

printOptions({"Attack the thief", "Attempt to flee"});

This is cleaner than the other version, and is reusable, so it will shorten your code. You can also have as many arguments as you wish, so if you want three options sometime, no problem.

Another problem you have is you are using namespace std;. This is bad, because later you may define your own namespace or use a different one that has some methods named the same, which could cause problems which could cause problems.

Your switch statements should not look like this:

switch (input) {
 case 1:
 searchBody();
 case 2:
 riverstead();
}

Instead, the case statements should be indented, like this:

switch (input) {
 case 1:
 searchBody();
 case 2:
 riverstead();
}

Also, I believe you have an error here. In C++, and most other languages, once a matching case statement is reached, you continue executing all lower case statements. If case 1: is matched and searchBody(); is executed, that means riverstead(); is also executed. You should add a break; statement to the end of each case block:

switch (input) {
 case 1:
 searchBody();
 break;
 case 2:
 riverstead();
 break; // unnecessary here because it is the last statement, but good practice.
}

I prefer if/else statements to switch statements, but switch statements are sometimes helpful.

Again, you have huge methods that should be split up into smaller blocks. I'm sure there are other things you can clean up, but this will be a good startm

There is a lot to be improved here. Number one is that you have huge blocks of code in super methods that handle entire moves. You should split that up into smaller logical groups for ease of reading, debugging, maintenance, and additions. One example I noticed is that you have this a lot:

cout << "\n 1. Attack the thief" << endl;
cout << "\n 2. Attempt to flee" << endl;

Maybe you should make a method to print the options:

void printOptions(std::vector<std::string> ops)
{
 for (int i = 0; i < ops.size(); i++) {
 std::cout << i + 1 << ". " << ops[i] << std::endl;
 }
}

Then, you can just call this:

printOptions({"Attack the thief", "Attempt to flee"});

This is cleaner than the other version, and is reusable, so it will shorten your code. You can also have as many arguments as you wish, so if you want three options sometime, no problem.

Another problem you have is you are using namespace std;. This is bad, because later you may define your own namespace or use a different one that has some methods named the same, which could cause problems.

Your switch statements should not look like this:

switch (input) {
 case 1:
 searchBody();
 case 2:
 riverstead();
}

Instead, the case statements should be indented, like this:

switch (input) {
 case 1:
 searchBody();
 case 2:
 riverstead();
}

Also, I believe you have an error here. In C++, and most other languages, once a matching case statement is reached, you continue executing all lower case statements. If case 1: is matched and searchBody(); is executed, that means riverstead(); is also executed. You should add a break; statement to the end of each case block:

switch (input) {
 case 1:
 searchBody();
 break;
 case 2:
 riverstead();
 break; // unnecessary here because it is the last statement, but good practice.
}

I prefer if/else statements to switch statements, but switch statements are sometimes helpful.

Again, you have huge methods that should be split up into smaller blocks. I'm sure there are other things you can clean up, but this will be a good startm

There is a lot to be improved here. Number one is that you have huge blocks of code in super methods that handle entire moves. You should split that up into smaller logical groups for ease of reading, debugging, maintenance, and additions. One example I noticed is that you have this a lot:

cout << "\n 1. Attack the thief" << endl;
cout << "\n 2. Attempt to flee" << endl;

Maybe you should make a method to print the options:

void printOptions(std::vector<std::string> ops)
{
 for (int i = 0; i < ops.size(); i++) {
 std::cout << i + 1 << ". " << ops[i] << std::endl;
 }
}

Then, you can just call this:

printOptions({"Attack the thief", "Attempt to flee"});

This is cleaner than the other version, and is reusable, so it will shorten your code. You can also have as many arguments as you wish, so if you want three options sometime, no problem.

Another problem you have is you are using namespace std;. This is bad, because later you may define your own namespace or use a different one that has some methods named the same, which could cause problems.

Your switch statements should not look like this:

switch (input) {
 case 1:
 searchBody();
 case 2:
 riverstead();
}

Instead, the case statements should be indented, like this:

switch (input) {
 case 1:
 searchBody();
 case 2:
 riverstead();
}

Also, I believe you have an error here. In C++, and most other languages, once a matching case statement is reached, you continue executing all lower case statements. If case 1: is matched and searchBody(); is executed, that means riverstead(); is also executed. You should add a break; statement to the end of each case block:

switch (input) {
 case 1:
 searchBody();
 break;
 case 2:
 riverstead();
 break; // unnecessary here because it is the last statement, but good practice.
}

I prefer if/else statements to switch statements, but switch statements are sometimes helpful.

Again, you have huge methods that should be split up into smaller blocks. I'm sure there are other things you can clean up, but this will be a good startm

Source Link
user34073
user34073

There is a lot to be improved here. Number one is that you have huge blocks of code in super methods that handle entire moves. You should split that up into smaller logical groups for ease of reading, debugging, maintenance, and additions. One example I noticed is that you have this a lot:

cout << "\n 1. Attack the thief" << endl;
cout << "\n 2. Attempt to flee" << endl;

Maybe you should make a method to print the options:

void printOptions(std::vector<std::string> ops)
{
 for (int i = 0; i < ops.size(); i++) {
 std::cout << i + 1 << ". " << ops[i] << std::endl;
 }
}

Then, you can just call this:

printOptions({"Attack the thief", "Attempt to flee"});

This is cleaner than the other version, and is reusable, so it will shorten your code. You can also have as many arguments as you wish, so if you want three options sometime, no problem.

Another problem you have is you are using namespace std;. This is bad, because later you may define your own namespace or use a different one that has some methods named the same, which could cause problems.

Your switch statements should not look like this:

switch (input) {
 case 1:
 searchBody();
 case 2:
 riverstead();
}

Instead, the case statements should be indented, like this:

switch (input) {
 case 1:
 searchBody();
 case 2:
 riverstead();
}

Also, I believe you have an error here. In C++, and most other languages, once a matching case statement is reached, you continue executing all lower case statements. If case 1: is matched and searchBody(); is executed, that means riverstead(); is also executed. You should add a break; statement to the end of each case block:

switch (input) {
 case 1:
 searchBody();
 break;
 case 2:
 riverstead();
 break; // unnecessary here because it is the last statement, but good practice.
}

I prefer if/else statements to switch statements, but switch statements are sometimes helpful.

Again, you have huge methods that should be split up into smaller blocks. I'm sure there are other things you can clean up, but this will be a good startm

lang-cpp

AltStyle によって変換されたページ (->オリジナル) /