3
\$\begingroup\$

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;
\$\endgroup\$
2
  • \$\begingroup\$ 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. \$\endgroup\$ – Aganju Feb 27 at 23:01
  • \$\begingroup\$ 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. \$\endgroup\$ – Toby Speight Feb 28 at 16:19
5
\$\begingroup\$

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.

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

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.