Login System In Batch

Question

Here I made a program in batch that detects all files with the .user extension. Then it allows the user to pick a username by entering the number associated with that username.

The code is messy, so I will explain.

@echo off
setlocal ENABLEEXTENSIONS ENABLEDELAYEDEXPANSION
:login_menu
cls
set x=1
set users=
cd "users"
for %%A in (*.user) do (
echo !x!. %%~nA
set users=%%~nA:!x!,!users!
set /a x=!x!+1
)
echo.
set /p ch=Select User: 
if "%ch%" == "" (goto login_menu)

for %%B in (%users%) do (
    for /f "tokens=1,2 delims=:" %%C in ("%%B") do (
    set userNumber=%%D 
    set userN=%%C
    if !ch! == %%D goto password
    echo BDEV: %%B
    pause
)
)
echo.
echo That user doesn't exist!
pause
goto login_menu

:password
cls
cd "users"
for /f "tokens=1,2 delims=:" %%E in (!userN!.user) do (set password=%%F)
echo Enter your password, !userN!
echo.
set /p password1=Password: 

if %password1% == %password% goto menu
echo.
echo That password is invalid!
pause
goto password

:menu
echo Hey! You're logged in as !userN!
pause

The variable x is going to be the number which the username will be associated with. The variable users Makes sort of a 'map' to usernames to the number associated with that username

In the first for loop, it gets all the files in the folder users. It echo's out all the usernames that the user can pick.

The second for loop goes into the variable users and separates the usernames with their respective numbers. For example: if I have the usernames admin:4,steve:3,john:2,jane:1 it will separate them into admin:4 and steve:3 and john:2 and jane:1.

The third for loop (which is in the second for loop) separates each username to number into separate variables. For example, if we have the username Collins with the number 3, it will put the username Collins in the userN variable and the number 3 into the variable userNumber.

It then checks what number the user selected.

The final for loop goes into the user file that the user has selected. So if the user selected admin it will go to the user file admin.user and find the password.

If the password is invalid, it rejects access. If it correct it allows access.

Is there any way to make it less like spaghetti code?

If you need more explanation, I will be happy to provide more information.

Answer 1

My main recommendation is that your variables have more descriptive names. While !x! may seem like a reasonable variable name for a counter that you're never going to use again, calling it something like !counter! makes your code easier for other people to maintain when there's a lot more of it.

I also got rid of ENABLEEXTENSIONS because that's enabled by default, and I moved cd users above :login_menu because if somebody enters an invalid user number and you're already in the users folder, your code will try to go into the users folder that's inside of the users folder and since there isn't one, you'll get an error.

To cut out that nested for loop, I stored the username map in an array. From there, you can determine whether or not a username is valid by if the variable exists.

Finally, I replaced the !s that you echoed with ^^!s so that they would be escaped. Because you have delayed expansion enabled, the last bit of your code would be displayed as HeyuserN because batch would consider ! You're logged in as ! to be a variable (because that's a valid variable name in batch).

If you could guarantee that you would never have more than ten users, you could build a string for a choice list to guarantee that the user would never enter an invalid user number, but I assumed this would be used by a large number of people.

@echo off
setlocal enabledelayedexpansion

cd users

:login_menu

set "user_counter=1"
for %%A in (*.user) do (
    echo !user_counter!. %%~nA
    set "users[!user_counter!]=%%~nA"
    set /a user_counter+=1
)
echo.

set /p "user_selection=Select user: "
if "%user_selection%"=="" goto login_menu
if not defined users[%user_selection%] (
    echo That user does not exist^^!
    pause
    goto login_menu
)
set "user_name=!users[%user_selection%]!"

:enter_password
for /f "tokens=1,2 delims=:" %%E in (!user_name!.user) do set "stored_password=%%F"
echo Enter your password, !user_name!
echo.
set /p "entered_password=Password: "

if "%entered_password%"=="%stored_password%" goto menu
echo.
echo That password is invalid^^!
pause
goto enter_password

:menu
echo Hey^^! You're logged in as !user_name!^^!
pause