Please offer criticism of my C++ code

[panzeramd]

Hey guys, i don't know if this is the right place for it but i would like you to offer negative or postive feedback of my little bit of code.

I am only up to page 60 ( out of 495 ) of my c++ book ( learn c++ in 24 hours ), but i came up with it by myself so i would feedback for encouragement.

#include <iostream.h>

int main()
{
int smallNumber = 10, largeNumber, TempA, TempB; // initialize smallNumber/largeNumber/TempA and TempB as inteagers and assign the value 10 to smallNumber

cout << "What is the value of smallnumber?:\t " << smallNumber << "\tis its value.\n"; // find the value of smallNumber
cout << "Please input a number to be assigned to largeNumber\n";
cin >> largeNumber; // input value for largeNumber
{
if (largeNumber <= smallNumber)
cout << "Sorry the value you input for large number is to low!, Please try again.\n";
cin >> largeNumber; // input value for largeNumber

if (largeNumber > smallNumber)
cout << "largeNumber is now bigger than smallNumber, Thank you very much!\n";
else
cout << "Sorry once again the value you input is smaller than smallNumber, Please try again.\n";
}

cout << "The value assigned to large number is:\t " << largeNumber << endl;

TempA = smallNumber; // Temp A has the original value of smallNumber assigned to it
TempB = largeNumber; // and TempB holds the original value for largeNumber

cout << "What value is smallNumber once it has been incremented:\t " << ++smallNumber << "\tis now the value of smallNumber.\n"; // prefix increment is used to icrement smallNumber
cout << "And what value is largeNumber when it is incremented:\t " << ++largeNumber << endl; // and prefix increment is also used to increment largeNumber

cout << "Find the original values for smallNumber and largeNumber - smallNumber's value is:\t " << TempA << endl; // finds the original value for smallNumber, which is stored in TempA
cout << "And the original value of largeNumber is:\t " << TempB << endl; // finds the original value of largeNumber, which is stored in TempB
cout << "Thank you very much!\n";
return 0;
}

I think its well commented an easy to follow but i would like to know what you guys think of it.

Also GCC warns about iostream.h being deprecated, so what do i use in its place.

 

[Saem]

Ack, I don't think that's too readable, but that might be more because you're not using the code tags. Throw the code in, code tags.

blah

 

[panzeramd]

#include <iostream.h>

int main()
{
int smallNumber = 10, largeNumber, TempA, TempB; // initialize smallNumber/largeNumber/TempA and TempB as inteagers and assign the value 10 to smallNumber

cout << "What is the value of smallnumber?:\t " << smallNumber << "\tis its value.\n"; // find the value of smallNumber
cout << "Please input a number to be assigned to largeNumber\n";
cin >> largeNumber; // input value for largeNumber
{
if (largeNumber <= smallNumber)
cout << "Sorry the value you input for large number is to low!, Please try again.\n";
cin >> largeNumber; // input value for largeNumber

if (largeNumber > smallNumber)
cout << "largeNumber is now bigger than smallNumber, Thank you very much!\n";
else
cout << "Sorry once again the value you input is smaller than smallNumber, Please try again.\n";
} 

cout << "The value assigned to large number is:\t " << largeNumber << endl;

TempA = smallNumber; // Temp A has the original value of smallNumber assigned to it
TempB = largeNumber; // and TempB holds the original value for largeNumber

cout << "What value is smallNumber once it has been incremented:\t " << ++smallNumber << "\tis now the value of smallNumber.\n"; // prefix increment is used to icrement smallNumber
cout << "And what value is largeNumber when it is incremented:\t " << ++largeNumber << endl; // and prefix increment is also used to increment largeNumber

cout << "Find the original values for smallNumber and largeNumber - smallNumber's value is:\t " << TempA << endl; // finds the original value for smallNumber, which is stored in TempA
cout << "And the original value of largeNumber is:\t " << TempB << endl; // finds the original value of largeNumber, which is stored in TempB
cout << "Thank you very much!\n";
return 0;
}

is that better?

I would like to find some tutorials on using the GUI of the windows flavour, so if anybody could point me to a guide i would be very grateful.

Thanks
Glen

 

[Ragemare]

is that better?

I would like to find some tutorials on using the GUI of the windows flavour, so if anybody could point me to a guide i would be very grateful.

Thanks
Glen

I'm not sure the formating of your code is quite right.

On the subject of GUI coding, you need to know how to use pointers, callback functions and objects before you tackle any of the windows GUI librarys, although the only WIndows GUI API I'v used was the one in the .NET library.

I think you should concentrate on doing console apps for now, try to implement a linked list, quick sort, text based battleship game etc.

 

[Davros]

only on page 60 ????

you mean it didnt take you 24 hours

 

[gokickrocks]

Also GCC warns about iostream.h being deprecated, so what do i use in its place.

<iostream>

...as for the code, it seems straightforward, although you dont really need to comment your cout statements since they seem self explanatory...and make sure you space out your statements, ie: within the if statement, the code inside should be tabbed

if (blah < blahblah)
blah blah blah blah blah

should be replaced with

if (blah < blahblah)
     blah blah blah blah blah

, however this is more of a personal preference but makes it easier to read

 

[Npl]

You should learn a coherent Code-Style as soon as possible (TempA -> tempA ). Best to avoid bad habits from the start

 

[Colourless]

You're using "\n" and endl

 

[Anarchist4000]

That is a real mess to read. Figuring out a coding style would probably help you a lot. I'm sure I messed something up in the code I posted but it will give you a better idea what you're trying to do. And how it's a lot more readable when you start commenting and spacing stuff out. There are a ton of different styles that are used in coding though.

void main()
{
  int small=10, large=0, tempa, tempb;

  //Gets value of large number
  while(large<=small)
  {
    cout<<"Enter the large number: ";
    cin>>large;

    //Insert insult here
    if(large>small)
      cout<<
  }

  //Correct number entered
  cout<<

  //Output some crap here
  cout<<
  cout<<
  cout<<

  return;
}

 

[Tokelil]

You should learn a coherent Code-Style as soon as possible (TempA -> tempA ). Best to avoid bad habits from the start

Agreed! A good naming scheme is very important when you want to read your code a half year from now. (And even more importent if others are ever going to read the code)
You dont have to follow the one posted by Npl, but it seems quite good. (And have quite a few things in common with normal Java stardand naming scheme, which is good IMO) Personally I like prefixing my variables with type (n for number, str for strings etc.) but people have different preferences...

 

[panzeramd]

Thanks for the input guy's.

My thought is that if want to learn c++, i want to be able to create readable code that can read by people and not just my self.

 

[NoteMe]

That is a real mess to read. Figuring out a coding style would probably help you a lot. I'm sure I messed something up in the code I posted but it will give you a better idea what you're trying to do. And how it's a lot more readable when you start commenting and spacing stuff out. There are a ton of different styles that are used in coding though.

void main()
{
  int small=10, large=0, tempa, tempb;

  //Gets value of large number
  while(large<=small)
  {
    cout<<"Enter the large number: ";
    cin>>large;

    //Insert insult here
    if(large>small)
      cout<<
  }

  //Correct number entered
  cout<<

  //Output some crap here
  cout<<
  cout<<
  cout<<

  return;
}

You are right, there is a ton. Like me...I would have written it like this:

[color=#FF6633]void[/color][color=#3333FF] main[/color][b][color=#663300](){[/color][/b][color=#FF6633]

    int[/color] small[b][color=#663300]=[/color][/b][color=#999900]10[/color][b][color=#663300],[/color][/b] large[b][color=#663300]=[/color][/b][color=#999900]0[/color][b][color=#663300],[/color][/b] tempa[b][color=#663300],[/color][/b] tempb[b][color=#663300];[/color][/b][i][color=#999999]

    //Gets value of large number
[/color][/i][color=#3333FF]    while[/color][b][color=#663300]([/color][/b] large[b][color=#663300] <= [/color][/b] small[b][color=#663300]){[/color][/b]
       cout[b][color=#663300] <<[/color][/b][color=#009900] "Enter the large number: "[/color][b][color=#663300];[/color][/b]
       cin[b][color=#663300] >>[/color][/b]large[b][color=#663300];[/color][/b][i][color=#999999]

     //Insert insult here
[/color][/i][color=#3333FF]     if[/color][b][color=#663300]([/color][/b]large [b][color=#663300] > [/color][/b] small[b][color=#663300])[/color][/b]
         cout[b][color=#663300] <<
     }[/color][/b][i][color=#999999]

    //Correct number entered
[/color][/i]    cout[b][color=#663300] <<[/color][/b][i][color=#999999]

    //Output some crap here
[/color][/i]    cout[b][color=#663300] <<[/color][/b]
    cout[b][color=#663300] <<[/color][/b]
    cout[b][color=#663300] <<[/color][/b][color=#3333FF]

    return[/color][b][color=#663300];
}[/color][/b]

Greeg Snooks once said something like: "For every coder, there is a coder that doesn't like his coding style." I think he was right....

[Edit] didn't notice before now...but it is missing some ;;;, guess I would have added them too..


Øyvind Østlund

 

[Frank]

panzeramd, I would suggest you to be your own critic. While your code is perfectly fine, especially when trying to learn how to do it, take a good look at it yourself, and ask yourself what you could do to improve it. And check your book to see how to do it, or Google around until you're satisfied.

That is the best way to do it.

 

[Grim Reaper]

I'd like to suggest that you think a bit more about your comments in the code. Beginners are always told that they should always comment their code so that it can be understood but it's a common mistake to misinterpret this advice.

Omit comments that merely echo the meaning of the programming language itself. I know that the language is new to you, so they may help you to remember what each line does, but it is better to avoid getting into bad habits. Use your C++ books to help you understand the programming language and leave comments to explain what you are doing in the "problem domain" and why you are doing it.

For example, the first line where you declare variables you have added a comment that doesn't tell us anything more than the line of C++ itself. IMHO, the only part of that line that may deserve a comment is your use of the number '10'. Why did you choose '10'? Was it an arbitrary choice or was there a particular reason for it? This is the kind of question that will occur to other programmers when they come across your code. This is exactly the kind of question that comments should answer. Expect people reading your code to understand the programming language - your comments should help them to understand your program. Too many comments that don't add any new information beyond that expressed in the code itself actually obfuscate your code.

Also on the subject of comments, don't feel you have to add them where they are redundant. This overlaps with the point I have just made, but if you ever find yourself tempted to comment a call to a function called 'DisplayMenu' with something like, "Display the menu", stop and think. If you choose names for your functions and variables carefully, this situation should occur quite often. It's actually a good sign when code appears to 'document itself' like that and you find that it's rare that you need a comment to provide additional information.

Like all good programming guidelines, this advice has exceptions. In the first case you may still want to document use of the programming language in an unusual way, when you use a rarely used language feature, or when the syntax of the language itself makes it difficult to understand what you are doing. If you stick with C++ you are likely to come across this latter situation at some point. ;-) In the second case, you may still want to add the 'redundant' comment for the sake of consistency, when its omission may cause confusion, or when your comments are used to create code documentation automatically.

Lastly, on the subject of code layout, I agree that everyone has their own preferences. I would concentrate on being consistent first - pick a standard from one of your books or somewhere on-line and use it. However, don't get too attached to it. Your own tastes are likely to change with time as you discover that some styles help you find or avoid errors and help you to understand particular constructions more easily. If you ever get a job doing programming you will often find that you have to switch style depending on the whims of your employer. At that point you can either evangelise your own style with the faint hope of winning them over, or give in and do it their way, using your own style when you code at home for your own pleasure.

Keep it up. It's a brave thing to post your work and invite criticism while you're still learning. Many of us would think twice about revealing our code when we've been programming for years. In some cases there's a good reason for that...

Regards,

Jim

 

[rwolf]

Try fitting as much as you can on a single line.