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

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;

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;
return 0;

• 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. Feb 27, 2021 at 23:01
• 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. Feb 28, 2021 at 16:19

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.

• 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? Feb 27, 2021 at 18:41
• I think the break is fine. An alternative would be to do something like do... while (c != '\r') , but personally I prefer the break. Feb 27, 2021 at 19:07