Coding Style Help

[Azurea]

Hey guys! I'm requesting constructive criticism on the code that I've written so that I might get a better idea of how to strengthen my code while keeping it short and sweet, or even just pointing out unnecessary things or things that I should have added.

My assignment was:

Write a program to get a user’s name in Last, First format. Then create a “user id” for the person consisting of his first initial and up to the first 7 characters of the last name.

What is your name? Montoya, Inigo
Your user ID: imontoya

My code was successful, but I'd like to know if there was a better way to have written it or make it shorter.

My code is:

/* User ID Maker.cpp : Creates a user ID made of the first letter of the user's first 
name and the first 7 letters of the user's last name.*/


#include "stdafx.h"
#include <iostream>
#include <string>
#include <cctype>
using namespace std;

int main(){
	
	string strName;	
	
	cout << "Enter your full name in last, first format to create a user ID: ";
	getline(cin, strName);

	const string::size_type stCOMMA_LOC = strName.find(',');

	if (stCOMMA_LOC != string::npos){ //Check that the user included the comma.
		
		char cFirstLetter = strName[stCOMMA_LOC + 2]; //The first letter should appear 2 spaces after the comma.
		string strUsername;
		strUsername.push_back(tolower(cFirstLetter));

		int nCounter = 0;
		
		//Add the first seven letters to the username.
		while (nCounter < stCOMMA_LOC && nCounter < 7)
			strUsername.push_back(tolower(strName[nCounter++]));

		cout << "Your username is: " << strUsername << endl;
	}

	else
		cout << "You did not enter your name in the correct form." << endl;

	return 0;
}

Thanks in advance for the help and criticism!

[csurfer]

Few things I found out :
1>Your stdafx.h header is not a compulsion.The program is running even without that header and giving the desired results.
2>The space in the input by the user after the comma is critical.If something like Montoya,Inigo would result in answer nmontoya rather than imontoya.So add a check to your program to check the space after the comma and display that input is not proper if that format is not found.

[siddhant3s]

Well, another "criticize my code" question. Good.
Here are my advices:

• Try not to implement the code which has been provided to you by the standard library. This way you will make shorter and elegant code.
• Avoid magic number: stCOMMA_LOC + 2 , stCOMMA_LOC && nCounter < 7 . Rather, put them under some const variable with a intuitive name so that a month later when you debug your code, you don't come shouting "Why the hell that 2 is been put there"
• Dumping everything in the global namespace by using namespace std; is not a good idea. Read this post for excellent review of what all options do you have,
• And yes, try to squeeze your code in 80 character width as people like me ( who run your code in 80-sized terminals) often find it harassing when our text editor wraps the long lines . Use \ character for doing this.

Here is my code, way bit shorter (8 statements) for nearly the same error checking:

#include <iostream>
#include <string>
#include <algorithm>
int main(){

    const size_t endlength=7;//the length of the lastname to be taken
    std::string firstname,lastname;

    std::cout <<"Enter your full name in last, first " \
    "format to create a user ID:";

    //  read till the first encounter of comma
    std::getline (std::cin, lastname,',');
    //  read the rest of the name
    std::cin >> firstname;

    //construct the username
    std::string username=firstname.at(0)+lastname.substr(0,endlength);
    //convert username to lower charachter
    std::transform(username.begin(),    \
                   username.end(),      \
                   username.begin(),    \
                   ::tolower);

    std::cout <<username <<std::endl;
}

Don't hesitate in looking up the reference for each of the function you don't understand:
substr ,transform,

std::transform(username.begin(), \
username.end(), \
username.begin(), \
::tolower);

Please see http://www.daniweb.com/forums/post853101-5.html

[Ancient Dragon]

[*]And yes, try to squeeze your code in 80 character width as people like me ( who run your code in 80-sized terminals) often find it harassing when our text editor wraps the long lines . Use \ character for doing this.

Sounds like a problem for the chaplain to me. :) My advice to you: come into the 21st century and get a newer terminal. I have not seen an 80-character width terminal in some 25+ years.

Also note that the \ characters at the end of lines 20-22 of the code you posted are unnecessary.

[Azurea]

Okay cool, thanks a lot for the help and references!

Well, another "criticize my code" question. Good.
Here are my advices:

• Try not to implement the code which has been provided to you by the standard library. This way you will make shorter and elegant code.

I think my problem is that I don't know many functions offered to me by the standard library. I think the only parts of the library I know about are string and cctype. Guess I'll be looking into that more next =).

[Azurea]

Sorry to double post (I wanted the thread to be pushed to the top), but regarding this line of code in siddhant3s's post:

std::getline (std::cin, lastname,',');

If the user doesn't enter a comma between the last and first names, the program hangs (I guess the function is stuck trying to find the comma?) If that's the case, is there some other way to read information from input only up to the comma? If not, will my method in the first post suffice (with the other tips given to me, of course)?

[siddhant3s]

>I have not seen an 80-character width terminal in some 25+ years.
I have seen them neither since birth, But my text editor, my shell window, and best of all, daniwebs code tag formatter are all 80-width wide. You would ( atleast I ) find annoying if you were to see a code like this;

{ ...nested blocks
            {
                    {
                            std::transform(username.begin(), username.end(), username.begin(), ::tolower);
                    }
            }
}
/*Or codes with long  comments following the code line*/


	if (stCOMMA_LOC != string::npos){ //Check that the user included the comma.
		
		char cFirstLetter = strName[stCOMMA_LOC + 2]; //The first letter should appear 2 spaces after the comma.
		string strUsername;
		strUsername.push_back(tolower(cFirstLetter));

		int nCounter = 0;
		
		//Add the first seven letters to the username.
		while (nCounter < stCOMMA_LOC && nCounter < 7)
			strUsername.push_back(tolower(strName[nCounter++]));

		cout << "Your username is: " << strUsername << endl;
	}

>I think my problem is that I don't know many functions offered to me by the
>standard library. I think the only parts of the library I know about are string and
>cctype. Guess I'll be looking into that more next =).
Thats the whole idea. When you would do a drill to search out the library reference your vocabulary of the standard algorithms and data structure will increase which would help you the next time you code.

>If the user doesn't enter a comma between the last and first names, the
>program hangs (I guess the function is stuck trying to find the comma?) If that's
>the case, is there some other way to read information from input only up to the
>comma? If not, will my method in the first post suffice (with the other tips given
>to me, of course)?
Well, do you wan't your user to enter a invalid input? The program doesn't hang but it is just waiting for the user to enter the comma. If you don't want this behavior, you could read the whole of the line at a time and then parse it perhaps using stringstreams. If the user wan't to finish entering data, he may press the end of the file character( Ctrl +D on Unix | On windows I think it is Ctrl+Z but I am not sure).
But that would cause an error as the .at() member function raises an exception( which is a good thing actually following the policy: "When you must fail, fail noisily and as soon as possible")

>Please see http://www.daniweb.com/forums/post853101-5.html
I think I have taken care about it. Look at the function:
I have used the non-templated tolower by using ::tolower and not tolower or std::tolower.