Bad programming practices

[CrazyCollegeGuy]

Hello! I'm taking an optional C++ course at my local uni, and the professor insists my code is full of so called "bad programming practices". I have some self-taught experience in C++, and I'm pretty sure my code is absolutely fine. Here's a quick and simple example of some pseudo-code I may write:

  #include <iostream>
  #include <stdio.h>
  #include <Windows.h>
  #include <stdlib.h>
  using namespace std;
  int main(){loopin:
  char lnameI[10],Isurnamel[10];
  printf("enter you're name:");
  cin>>lnameI;
  printf("enter your surname:");
  cin>>Isurnamel;
  char _fullname[20];
  for (int i=0;i<10;i++){_fullname[i]=lnameI[i]}
  for (int i=10;i<20;i++){_fullname[i]=Isurnamel[i-10]}
  printf("your full name is ");
  for (int i=0;i<20;i++){cout<<_fullname[i];}
  system("pause");
  char ans;
  printf("do it again?")
  cin>>ans;
  if (ans=='y'){goto loopin;}
  else {goto endin;}
  endin:
  return 0;}

What do you think? Is my code fine or should I listen to my professor?

[48T7M4Gy]

Well, the bad news is he's right unfortunately. But all is not lost so don't lose hope :)

- you have no comments to indicate what this program does
- goto's are bad
- printf, char arrays are C, not C++
- system commands are not good, but everybody knows why they are included
- your program should be indented
- you need to write clear code using whitespace
- use variable names that assist the program in being self-documenting
- writing code on a single line is cute but not good practice for newbies, or just about anybody

Depending what your prof is saying it is probably a good idea to listen to the advice and go to lectures.

Post the question here and you can see how people here would tackle it.

BTW are you sure you've used the term 'pseudocode' correctly?

[48T7M4Gy]

Maybe something like this is what your program is supposed to do.

#include <iostream>
#include <string>

using namespace std;

int main(){
    
    const int SIZE = 10;
    
    // SETUP STORAGE ARRAYS
    string first_name[SIZE], surname[SIZE], fullname[SIZE];
    
    char answer = 'y';
    int count = 0;
    
    do{
        // CHECK STORAGE IS AVAILABLE
        if (count > SIZE)
        {
            cout << "Too many names\n";
            break;// end
        }
        
        cout << "enter you're name: ";
        cin >> first_name[count];
        
        cout << "enter your surname: ";
        cin >> surname[count];
        
        // JOIN UP THE NAMES
        fullname[count] = first_name[count] + "***" +surname[count];
        
        cout << "your full name is " << fullname[count] << '\n';
        count++;
        
        // ANY MORE NAMES STILL TO ENTER
        cout << "do it again? ";
        cin >> answer;
    } while (answer == 'y');
    
    // FINISH UP BY LISTING OUT THE DETAILS ENTERED
    for(int i = 0; i < count; i++)
        cout << first_name[i] << ' ' << surname[i] << " Full name is: " << fullname[i] << '\n';
    
    return 0;
}

enter you're name: bob
enter your surname: smith
your full name is bob***smith
do it again? y
enter you're name: judy brown
enter your surname: your full name is judy***brown
do it again? n
bob smith Full name is: bob***smith
judy brown Full name is: judy***brown
Program ended with exit code: 0

[CrazyCollegeGuy]

Thank you for your answers!

I'm a little confused about whitespace and blank lines, as Google's C++ Coding Standard states that programmers should fit as much code on the screen as possible. Also, I've heard that C code is much better than C++ code performance-wise, so one should use the former whenever possible.

BTW are you sure you've used the term 'pseudocode' correctly?

Not really, sorry :(

[dhayden]

Prefer to use C++ strings over char arrays. Your code's behavior is undefined for a name like Elizabeth Campodonico.

Line 12: Don't start variable names with underscores. They are reserved by... the implementation? The library?

Don't use goto unless absolutely necessary (which it almost never is).

Lines 15 & 16: Don't mix C and C++ I/O.

Lines 12, 13, 14 & 16: Don't use magic constants.

Lines 7 & 12: name variables consistently. What is the l in lnameI and Isurnamel? What are the I and l reversed in these names? Why isn't the full name Ofullnamel or ifullnameO?

[48T7M4Gy]

@Crazy,

On whitespace and lines, just imagine you are typing a letter and you want someone to read it without too much trouble and you've got plenty of paper.

The computer/compiler couldn't care less but humans don't like it all bunched up and you have plenty of memory space so don't skimp. The other thing is to write the code in chunks and when you go to the next separate phase just put in a line to space it out as mine does.

Google coding is OK and mentions this stuff. There are lots of these standards around and they vary so concentrate on the basics, whitespace, comments, variable naming. Unfortunately fitting as much on the screen is not right and I doubt whether Google would say that. Correct me if I'm wrong and I'm sure you will. :)

Others will comment in voluminous detail about C vs C++ so stick around. Short answer is, that notion is BS. Code in C if you like and there is plenty of legacy code to work on as a career path but it isn't based on performance.

PS Just to allay any fears yours isn't pseudocode, yours is 'full-blown' C with smatterings of C++ (cout etc). But it raises a separate hint for your programming endeavours. Pseudocode is a very good planning tool and should be used (students rarely do). By planning out you code strategy instead of lurching from code line to code line can save hours, days, weeks and more of sweat.

Anyway, good luck with it, feel free to come back if you have any questions. I'll double check Google myself. :)

[jonnin]

If you are writing a code book in print, then save space is good.
but if you are not, all your {} need to line up in the same text column matched, one statement per line with few exceptions (simple initializers are ok to lump, for example), comments, and empty lines as spacers between concepts/functions/etc.

two immediate issues are C headers <cstdio> and <cstdlib> are c++, .h is C.
The difference is the c++ ones namespaced them.

you should use string, not char array, for anything serious.
you should use cout unless there is a reason not to. I use printf when doing a lot of doubles, but its also C. Mixing a lot of C into the code without reason is messy, even though it works. If you have a good reason, that is another story.

system is dangerous. Its fine to play with, but it is a know security flaw as its super easy to inject other commands into it with a virus or hack, to get the program to do something else. I could drop a nasty "pause.exe" on your drive and your program would run it for me. Not a concern for schoolwork, of course.

goto in c is one of the better goto statements out there, but its still awful. It has one accepted use that I know of, that is busting out of deeply nested loops if there is a critical failure or early exit condition. Otherwise its as taboo as global variables

As yoda once said.. you must unlearn what you have learned ...

all that aside, working code that is a little messy > pretty code that is busted.
You demonstrate skill and understanding here. If you keep reading the code posted here, youll keep seeing better ways to do stuff. Listen to your professor.