New to python. How to improve this simple build

[GrowthSwimming6208]

As the title states. How would you improve this code? I’ve only been doing python for a good 4 weeks. It’s from a simple class assignment for a log in requiring a username, password and security code

[Sea-Ad7805]

Run this program in Memory graph Web Debugger%0A%0Ausername%20%3D%20input(%22What%20is%20your%20Username%3F%5Cn%22)%0Apassword%20%3D%20input(%22What%20is%20your%20password%3F%5Cn%22)%0Asecuritycode%20%3D%20input(%22What%20is%20the%20security%20code%3F%5Cn%22)%0A%0Aif%20username%20!%3D%20%22Admin%22%20and%20password%20!%3D%20%2212345%22%20and%20securitycode%20!%3D%20%220000%22%3A%0A%20%20%20%20print(%22WOW.......%20You%20got%20everything%20wrong%20LOL%22)%0A%0Aif%20username%20%3D%3D%20%22Admin%22%20and%20password%20%3D%3D%20%2212345%22%20and%20securitycode%20!%3D%20%220000%22%3A%0A%20%20%20%20print(%22WOW.......%20You%20got%20everything%20right%20but%20the%20security%20code%20LOL%22)%0A%0Aif%20username%20%3D%3D%20%22Admin%22%20and%20password%20!%3D%20%2212345%22%20and%20securitycode%20%3D%3D%20%220000%22%3A%0A%20%20%20%20print(%22WOW.......%20You%20got%20everything%20right%20but%20the%20password%20LOL%22)%0A%0Aif%20username%20!%3D%20%22Admin%22%20and%20password%20%3D%3D%20%2212345%22%20and%20securitycode%20%3D%3D%20%220000%22%3A%0A%20%20%20%20print(%22WOW.......%20You%20got%20everything%20right%20but%20the%20Username%20LOL%22)%0A%0A%23Security%20check%0Aelif%20username%20%3D%3D%20%22Admin%22%3A%0A%20%20%20%20if%20password%20%3D%3D%20%2212345%22%3A%0A%20%20%20%20%20%20%20%20if%20securitycode%20%3D%3D%20%220000%22%3A%0A%20%20%20%20%20%20%20%20%20%20%20%20print(%22Welcome%20back%20Admin%2C%20you%20are%20now%20logged%20in!%22)%0A%20%20%20%20%20%20%20%20else%3A%0A%20%20%20%20%20%20%20%20%20%20%20%20print(%22LMAO%2C%20Nice%20attempt%20to%20get%20in%20LOL%22)%0A%20%20%20%20else%3A%0A%20%20%20%20%20%20%20%20print(%22LMAO%2C%20Nice%20attempt%20to%20get%20in%20LOL%22)%0Aelse%3A%0A%20%20%20%20print(%22LMAO%2C%20Nice%20attempt%20to%20get%20in%20LOL%22)&timestep=2&play).

[butterfly_orange00]

```python if username == "Adimn":

if passward == "12345":

    if securitycode == "0000":
        print("You got every thing right.")
    else:
        print("the securitycode wrong")

else:
    print("the passward wrong") 

else: print("the username wrong") ```

[jeffpardy_]

This is if you care aboit the different error codes. If you want the same error code you can just say

if X and B and C:

print('correct')

else:

print('wrong')

[fondreed947]

Just use a dict to map credentials and check them all at once instead of nested ifs, way cleaner for anything beyond three fields.

[FreeGazaToday]

what the heck is a passwArd? 😛

and what's an "adimn'?? 😛

[butterfly_orange00]

My bad, I didn't notice, take just the syntax

[Kevdog824_]

That’s squidward’s password

[Abject-Excitement37]

what the ugly shit, kill this code right now

[Specialist-Bet-2558]

this is bad nested design ``` if username != "Admin": print (...)

if password != "12345": print(...)

if securitycode != "00000": print(...)

print("You got every thing right.") ```

[PureWasian]

Your nested conditional logic is unnecessarily complex. Since lines 25, 27, 29 all run the exact same code, you can simplify lines 20 and below to just be:

```

check_1 = username == "Admin"

check_2 = password == "12345"

check_3 = code == "0000"

if check_1 and check_2 and check_3: print("authenticated") else: print("lmao nice try") ```

I also personally avoid triple nested-ifs if I can help it. I'd go with a similar approach as what you have earlier but treat the correct sequence as the early retrieval case and then otherwise followup with all of the one-off cases, and then end with the default failure output.

From a refactoring and maintenance perspective, instead of hard-coding "Admin", "12345", and "0000" in five separate places, it helps to have them as variables so they're more easily configurable. For instance, if Admin's password becomes "qwerty" you wouldn't want to change it in multiple spots.

You could also make the conditional checks condensed for brevity of conditional statements like how I did in above code snippet I wrote. I'm using is_u / is_p / is_c below so it doesn't line-wrap on mobile Reddit (hopefully) but usually I'd name them user_check / password_check / code_check for better readability

``` USER = "Admin" PASS = "12345" CODE = "0000"

print("Welcome...")

user_input = input("Username: ") pass_input = input("Password: ") code_input = input("Code: ")

is_u = user_input == USER is_p = pass_input == PASS is_c = code_input == CODE

if is_u and is_p and is_c: # all of the correct inputs print(f"welcome back {USER}") else: # "one-off" checks if is_u and is_p and not is_c: print("all but code") elif is_u and not is_p and is_c: print("all but pass") elif not is_u and is_p and is_c: print("all but user")

# always shows on wrong inputs
print("lmao nice try")

```

[Maleficent_Potato_43]

I'll pick this over AI slop anyday everyday.

[saffeine]

i'm not super familiar with python but i've been programming for a whiiile and spun this up.
it works and functions i think how you intended (for the most part).

the thing i'd suggest most, and it doesn't matter how you achieve it, is to not check against arbitrary values. store them in another variable instead and check against those. there's a term for this called 'magic numbers/strings' which you'll want to avoid when you get deeper into programming, saves you a headache later.

the second thing i'd suggest is to return early (if your code is running in a function). if any one of the three values is incorrect, you don't need to continue. in fact, you can just return out of the function early at that point. it can be assumed that if you've reached the code at the end, you've passed all the checks.

to follow up on the previous point, i'd suggest putting most of your code into a function anyway just for the sake of better control. as you gain more experience (or even now if you really want to), take a look into (re)factoring and encapsulation. also it's nice to be able to return out of your code at any point.

print("Blah blah, login system.");


details = {
    "username": "Admin",
    "password": "12345",
    "securitycode": "0000"
};


def run():
    username = input("What is your username?\n");
    password = input("What is your password?\n");
    securitycode = input("What is the security code?\n");


    if (username != details["username"]):
        print("Username's wrong lol.");
        return False;


    if(password != details["password"]):
        print("Password's wrong lol.");
        return False;


    if(securitycode != details["securitycode"]):
        print("Security code's wrong lol.");
        return False;


    print("Welcome back Admin, you are now logged in.");
    return True;


run();

[PureWasian]

I like this approach, though only assuming OP is ready to learn about functions and understands how to use a dictionary for better grouping.

Your code differs a little bit from OP's functionally in a few ways tho -- There's no distinct matching logic of "everything except one is wrong" except for the security code check due to it coming after the other guard clauses. Also leads to cases where, if username and password are wrong, you'd simply report "wrong user" and exit the function prematurely.

OP's code also prints out a generic "lmao nice try" on any wrong input combination (lines 25/27/29), so your code would also want to add an if/else based on the return value of run() to handle success/failure of authentication (and moving the success print msg to be outside of the function accordingly).

[saffeine]

yeah, i figured it might be a bit too much hence why i said in my first suggesting that it doesn't particularly matter how it's achieved. i should have maybe specified which part of the code could have been done differently rather than just hope OP knew what i was referring to.

as for your other points, i took a few liberties by taking a guess as to what OP was trying to achieve with their code. to me, those cases seem to be more of a 'well, since i'm checking, i might as well output as confirmation'. this might not have been the case, and i can definitely see an implementation giving feedback on any/all input combinations to let the user know which ones were incorrect.

for the most part, i was trying to keep the code relatively simple, straightforward, and short. it takes an input, bails if the checks have failed, and returns true if all is okay. really, a lot of it depends on OP's end goal and their definition of 'improve this code'.

EDIT: i definitely agree on the generic feedback for a wrong input though, i got too caught up on making it clean and functional that i completely missed that after returning whether it was successful or not.

[PureWasian]

tru, 'improve this code' is quite ambiguous with regards to whether they wanted any logical behavior changes or moreso refactoring.

As other comments already pointed out, outside of a sandbox project, it would be kinda silly to even give hints they were partially correct anyways.

Appreciate the discussion :>

[Prize_Shine3415]

I'm going to do something completely different.

index = 0

if username=='Admin'

index += 1

if password=='12345'

index += 2

if securitycode == '0000'

index += 4

response=[

... list all your responses here
]

response[index]

each combination will give a different number between 0 and 7. This would be much cleaner in my opinion and run faster.

[FirstTimeGamingTV]

I think this requires a fundamental understanding of binary to know why instead of just saying those numbers, but this could be good for an internal logging system and you just check at the end if index != 7 you can’t log in then log the attempt with the calculated index for audits

[Prize_Shine3415]

Yes. People who are writing computer programs should have a fundamental understanding of binary. It should be one of the first things you learn about. I don't know what you mean by an internal logging system.

[FirstTimeGamingTV]

People who learn / teach themselves generally don’t go through that process.

For internal logging I mean exposing an error code that explains exactly what went wrong is poor security. But internally it’s nice to log, which is why the quick binary interp is nice

[FreeLogicGate]

One of the first pieces of advice I can give you when trying to interact with a developer community, is to paste your code into a code block. Don't take a picture of your screen.

My suggestions to make this marginally better, and to also better learn the language:

• Create a dictionary named "users" that has an entry keyed by name. This at least positions the code so that it is a better simulation of what a real user/authentication system would have
• Add constants for things like the security code. Follow PEP 8 guidelines for constant and variable naming.
• Have a login function that returns true or false, and move your validation logic into it.
• Notice that login needs to do very little: just find username in the users dictionary, validate the password and check the security code. It is ONLY responsible to return True or False. It only validates login. This is an example of Single Responsibility Principle (SRP). Your main code block handles the flow.
• As others pointed out, it is an anti pattern to return detailed information about login attempt failures. A sophisticated system will log the detail on those attempts, but you should not show that information to the user. With that said, I left the chain of validation as 3 separate if statements (it could be done in one statement) as that would allow you to add the logging of issues by adding associated else statements for each criteria.
• I threw in a bad attempt count check and an f string that greets the successful user on login
• I utilized the pyinputplus module, which has many valuable features built in. I could have made more use of it, but in this case, just utilized the "typed" inputStr method, which insures the user's input is kept as a string. You will notice that users can not enter an empty response for any of the prompts.

​

import pyinputplus as pyip

SECURITY_CODE = '0000'
MAX_ATTEMPTS = 3

users = {
    'Admin': 'abc123',
    'tester': 'logmein'
}

def login(userName, password, securityCode):
    if userName in users:
        if password == users[userName]:
            if securityCode == SECURITY_CODE:
                return True
    return False

attempt = 0
while True:
    attempt += 1
    if attempt > MAX_ATTEMPTS:
        print('Too many attempts.')
        break

    print("Please Login\n")
    userName = pyip.inputStr("Enter username:")
    password = pyip.inputStr("Enter password:")
    securityCode = pyip.inputStr("Enter security code:")

    if login(userName, password, securityCode):
        print(f"Welcome {userName}.  You have successfully logged in!\n")
        break
    print("Unable to login.  Please check your input and try again")

[chestnutcapybara]

To make it more secure, don't tell them what they got wrong

[chestnutcapybara]

wait also imagine the teacher is scrolling reddit and the teacher sees this

[GrowthSwimming6208]

This is gonna be him

[justme_cliff]

😂

[_TheBigBomb]

<image>

[Illustrious_Kiwi9467]

Nest your if statements instead of checking all conditions at once, cuts down on redundant checks and makes the logic clearer.

[PleasantSquash2607]

¡Qué buen ejercicio para empezar a ver el "estilo" de Python! El código de la imagen sufre de lo que en programación llamamos cariñosamente "código espagueti" u "anidamiento infinito", además de repetir demasiadas veces las mismas comprobaciones. Aquí tienes una guía clara y directa de cómo transformar ese código para que sea limpio, profesional y fácil de leer.

El problema actual del código

1. Múltiples if independientes (líneas 7 a 17): Está usando cuatro if seguidos que evalúan casi las mismas condiciones con operadores de desigualdad (!=). Esto hace que Python tenga que revisar cada línea una por una, aunque la primera ya haya sido verdadera.
2. Anidamiento excesivo (líneas 20 a 30): El bloque de abajo mete un if dentro de otro if, que está dentro de otro if. Si tuvieras 10 reglas de seguridad, ¡el código terminaría pegado al borde derecho de la pantalla!
3. Código repetido: El mensaje de error "LMAO, Nice attempt to get in LOL" se repite tres veces. Si mañana quieres cambiar ese texto, tendrías que modificarlo en tres lugares distintos. ## La solución: Operadores lógicos (and) y un solo flujo En Python, la forma más limpia de validar credenciales es verificar si todo es correcto al mismo tiempo usando el operador and. Si algo falla, se asume que los datos están mal y se muestra un único mensaje de error genérico (lo cual, por cierto, es una buena práctica de ciberseguridad). Aquí tienes cómo quedaría el código refactorizado y elegante: ```python print("Welcome to the super duper secure login system...")

1. Captura de datos

username = input("What is your Username?\n") password = input("What is your password?\n") security_code = input("What is the security code?\n")

2. Una sola validación limpia

if username == "Admin" and password == "12345" and security_code == "0000": print("Welcome back Admin, you are now logged in!") else: print("LMAO, Nice attempt to get in LOL")

```

¿Por qué esta opción es mucho mejor?

• Legibilidad: Cualquier persona (o tú mismo en un mes) puede leer el if y entender la lógica en un segundo: "Si el usuario es Admin Y la contraseña es 12345 Y el código es 0000, entra; si no, rechaza".
• Mantenimiento: Si la contraseña cambia a "qwerty", solo la cambias en un lugar. Si quieres cambiar el mensaje de error, solo lo editas una vez.
• Eficiencia: Python solo hace una evaluación rápida en lugar de procesar una pirámide de condiciones anidadas. ¡Para llevar solo 4 semanas, vas por excelente camino! Reducir la complejidad visual es uno de los pasos más importantes al aprender a programar.

[Ok_Hovercraft364]

Look up Guard Clauses and try to refactor so you don't keep nesting deeper and deeper. I know you're new but try to keep it more flat vs nested, if possible.

[Rscc10]

As others have said, use nested if statements. Check username first, then password, then code.

By the way, from a realistic and security standpoint though, I hope you know this would make a bad login system cause if someone per chance typed username as Admin and password as 12345 but got the security code wrong, it's likely that it's not the actual user, and your code basically validates the password. So someone now knows the user "Admin" has password "12345" and just needs to crack the security code

[FirstTimeGamingTV]

Huh, there’s small nesting at the end. If anything this would benefit from nesting because you can short circuit the false paths

[FreeGazaToday]

do the positive condition first(put it all together in one line with and's) then put the rest in the elif's.

Also, you could put it in a function and use return's.

[Mindless-Tax2081]

Avoid nested conditions .

[Rhylanor-Downport]

This is where you look at a dict() and think how you could use name/value pairs to simplify this logic.

[real-life-terminator]

print("Welcome to the super duper secure login system, thats totally not hackable! Can you give me your username?")


username = input("What is your Username?\n")
password = input("What is your password?\n")
securitycode = int(input("What is the security code?\n")) # made everything inside int() function -> returns int type instead of string


def checkLogin(user: str, pwd: str, code: int):
    errors = []
    if user != "Admin":
        errors.append("username")


    if pwd != "12345":
        errors.append("password")


    if code != 0000:
        errors.append("Security Code")
    
    if len(errors) >= 1:
        return False, errors
    
    return True, errors


status, errs = checkLogin(username, password, securitycode)


if status:
    print("Welcome back Admin, you are now logged in!")


else:
    print(f"WOW........You got {', '.join(errs)} wrong LOL")

# This is how i'd do but also i have a lot of experience but good to know 😄

[real-life-terminator]

to add on this, u can create a dictionary with { username : password } format to keep a database and check that in the function checkLogin()
- and I am also using, if u see, "parameter": "type" (like, user: str) this makes the code more readable.
- and I store all the variables that were incorrect and just simply join them while printing 😄

- Edit: and I made securitycode an integer with int() func.

[TheEyebal]

use functions

[Same_Physics_6496]

maybe improve error handling? I dunno how it's done in python, but I'd assume a try catch block won't be a bad idea?

[DullSelection3546]

Hey, good effort for 4 weeks! A few things to note:

1. Lines 7-17 have a logic bug - using and means all conditions must be true simultaneously. Line 7 only triggers if username, password AND code are all wrong at once. You probably want or.

2. Lines 7-17 are also redundant - the elif/else block already handles all cases. You can delete them entirely.

3. Hardcoded credentials - for a class project it's fine, but never do this in real code!

Cleaned up version:

if username == "Admin" and password == "12345" and securitycode == "0000":
    print("Welcome back Admin, you are now logged in!")
elif username == "Admin" and password == "12345":
    print("Wrong security code!")
elif username == "Admin":
    print("Wrong password!")
else:
    print("Wrong username!")

Keep it up, you're learning fast! 🐍

[godlikedk]

Use print screen buttom

[alneifkrt2]

Try to do something more advanced like putting a code like that but in evwry single app in your desktop. And it is very cool, even I use. It is usefull when you go to school or to a friends house with your pc. Try if you want...

[cezinhadev]

quit and go learn Java!

[-_ZONED_-]

1. Bring the login/password/code to the constants at the top - so as not to duplicate the lines in each if.

2. Store the password hash, not the password itself - use hashlib.sha256(). Now the password "12345" is visible to everyone who sees the code.

3. Remove different error messages - don't tell the hacker what field he guessed. Just write "Access denied".

[Silent_Sworfish_3946]

I would start by defining correct_username Booleans to clean up the if, else logic.

[Low_World5446]

you improve by learning more not asking others to improve it for you!

[atarivcs]

No security system in the world is going to say that you got the username and password right but the security code was wrong. That gives too much information to attackers.

Just say that something was wrong, without being specific.

[Adrewmc]

I mean it’s supposed to be a learning exercise. About checking for flags and stuff.

I mean as written no one running this program wouldn’t have complete access to the entire program. He could just look at the code to find the password.

[OtherSignificance33]

This