How can I make this C++ code to obtain passwords more efficient?

Question

The comments in the code kind of explain it. Backspace characters only move cursor back and don't erase output so I need to overwrite them with ' ' to delete them.

#include <iostream>
#include <string>
using namespace std;
const string PASSWORD = "PASSWORD";
int main(int argc, char** argv) {
string userkey = "";
char c;
int keystrokes = 0;
cout<<"Enter passkey: ";
while(true){
 if(_kbhit()){
 c=_getche();
 if(c=='\r'){//check if user hit enter, _getche() records enter as \r
 cout<<'\n';
 break;
 } 
 else if(c =='\b'){//check if user typed backslash, overwrite character there
 cout<<' ';
 if(keystrokes==1){ //makes sure userkey is cleared
 userkey = "";
 keystrokes--;
 cout<<'\b';
 }
 else if(keystrokes!=0){
 userkey = userkey.substr(0,userkey.length()-1);//cuts out last char of userkey
 keystrokes--;
 cout<<'\b';
 }
 }
 else if (c!='\b'){
 cout<<"\b*";//replaces enter char with'*'
 keystrokes++;
 userkey+=c;
 }
 }
}
//cout<<userkey;
if (userkey!=PASSWORD)
 return 0;

• Aganju: Do you understand that a hardcoded password gives very little security? Anyone can simply open the executable in Notepad and see the password, as it will be a part of the binary.
• Toby Speight: I don't see where you defined those functions _kbhit() and _getche(), since you don't present them here, and don't have any includes for them. But note that identifiers beginning with _ are reserved for the implementation, so you should be using more portable names.

Answer 1

Regarding your program's efficiency, remember that premature optimisation is the root of all evil. Looking at your program, I can tell that your program will be IO bound - meaning that the computer will spend longer waiting for the user to input the data than actually executing your program. Moreover, functions like std::cout which deal with output, include inevitable slow system calls, which will be far costly in time than your algorithm. So performance should not be a concern here ! :)

The only thing I would say about your program efficiency is const string PASSWORD = "PASSWORD"; which will use unnecessary copying and heap allocation. It is better to preserve the original type of the string like so:

const auto PASSWORD = "PASSWORD"; (Here auto (assuming c++11) deduces the type for you, but the real type of the string is const char* if you want to know)

CODE REVIEW

These are the style/ readability points I would make on your code.

• Use proper indentation (this might due to copy pasting on codereview so I don't blame you for that)
• Do not use using namespace std - this will lead to naming collisions and all sort of horrible errors. Better to use the std:: every-time instead.
• You might consider to put the variable PASSWORD inside main as a static const variable, this will limit the visibility of PASSWORD.
• Use a better variable name than c.
• If your program gets bigger, consider wrapping your algorithm inside a function, this might help to make the main function a bit neater.

That's all! Well done, I would say generally this is very well written, readable c++ code.

• GGrylls: Thank you! I'm new to C++ and this was very encouraging. Thanks for all of the suggestions, I'll change the variable name and definitely move this into a function, it's part of an encryption/decryption program. Would you say that using a break statement here is a good idea, or is there a better way to write the loop?
• SomeProgrammer: I think the break is fine. An alternative would be to do something like do... while (c != '\r') , but personally I prefer the break.