Hi,
Here goes with a list of things that might be improved:
1 2 3 4 5 6 7
|
#include "stdafx.h"
#include <stdio.h>
#include <conio.h>
#include <iostream>
#include <limits>
#include <string>
using namespace std;
|
1. Why
stdio.h and
iostream ?
2.
conio.h isn't portable - investigate using
ncurses instead.
You don't seem to use anything from conio anyway. Maybe the
_getche(); ? There is an article about this on this site, look at the top left of this page. Also reference material and a tutorial.
3. Don't have line 7. put
std:: before each
std thing. There is lots written as to why this so. Google.
9 10 11 12 13 14 15 16 17 18 19 20 21 22 23
|
int main(){
char Z;
string L;
string X;
string YES = "YES";
string NO = "NO";
string CONTINUE = "CONTINUE";
string EXIT = "EXIT";
string WAKE = "WAKE";
string DODGE = "DODGE";
string FALL = "FALL";
string DAGGER = "DAGGER";
string RAFT = "RAFT";
string HUT = "HUT";
string REAWAKEN = "REAWAKEN";
|
4. Always initialise your variables to something when declaring them. This is often one of the biggest sources of error. Use meaningful names for your variables, the name itself should explain what the variable is for. Use comments to explain expected ranges of values etc.
string YES = "YES";
You have a lot of these. This will fail if the user enters a different combination of upper & lower case - e.g. "Yes". Investigate the use of
enum and the
switch statement. Also search on this site for "bool controlled while loop". Make sure the switch has a
default: case. IMO this is much better than an infinite do loop. Each
case should call a function. Note that switch works with constant
int or
char, so one can't use them with strings. Testing against input strings is a misnomer anyway, so have the user input single chars is the easiest thing to do. Use the
toupper or
tolower function to make the logic easier.
5. Make more use of functions. Each time you print a bunch of text, call a function which does that. It will make your code much more readable. There is a bit of an unwritten rule that functions should be less than 40 Lines Of Code (LOC), some even go for even less.
52 53 54
|
if (Z == 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 'A', 'B', 'C', 'D', 'E', 'F') {
cout << "0101000010100110010101000 \n"
"...translating01000101010101001010 \n"
|
6. Does that work? The comma operator doesn't do what what you think it does. If
Z was an uppercase
char you could do this:
1 2 3 4 5 6
|
if ( (Z >= '0' && Z <= '9') || (Z >= 'A' && Z <= 'F') ) {
std::cout << "Hex Digit\n";
}
else {
std::cout << "Not a Hex Digit\n";
}
|
Or use a
switch again.
7.
If you insist on using a do loop (I personally really dislike them), then always put the
while part on the same line as the closing brace of the
do part, so it doesn't look like a while with a null statement:
} while (condition); // end of do while loop
There you go some stuff to think about :+)