SS64 Forum

Nexusfactor

Member

Registered: 15 Dec 2015

Posts: 8

Avoiding Spaghetti Code

I've been reading how to avoid Spaghetti Code in batch files (Sorry, can't post link, forum rules). It's off of Rob van der scripting pages, How to avoid spaghetti code.

In the example of what spaghetti code is, I realized that the batch file that I use when I logon almost fits this example. Could someone please help me make my batch file more robust, and not have spaghetti code?

code:

@ECHO OFF
CLS


:MENU
echo Welcome %USERNAME%

echo 1 - Start KeePass
echo 2 - Backup
echo 3 - FireFox
echo 4 - Exit

SET /P M=Please Enter Selection, then Press Enter:

IF %M%==1 GOTO StarKeePass
IF %M%==2 GOTO Backup
IF %M%==3 GOTO FireFox
IF %M%==4 GOTO :EOF
GOTO MENU


:StarKeePass
SET keePass="%USERPROFILE%\KeePass\KeePass-2.30\KeePass.exe"
SET kdb="%USERPROFILE%\KeePass\PasswordDatabase\PasswordDatabase.kdbx"

echo I'll start KeePass for You
START "" %keePass% %kdb% 

GOTO MENU

:Backup
SET backup="%USERPROFILE%\backup.bat"
call %backup%

GOTO MENU

:FireFox
cd "C:\Program Files (x86)\Mozilla Firefox\"
start firefox.exe

GOTO MENU

Shadow Thief

Member

Registered: 12 Jul 2012

Posts: 205

Re: Avoiding Spaghetti Code

If that's your entire code, it's not complex or convoluted enough to be considered spaghetti code. That said, there are a couple of ways you can reorganize your code to completely avoid using goto altogether, especially if you have the option of using the choice command.

If you don't want to use goto at all, you can call the script again when you're done with it.

@echo off

cls
echo Welcome %USERNAME%

echo 1 - Start KeePass
echo 2 - Backup
echo 3 - FireFox
echo 4 - Exit

:: choice allows users to select one item from a list of choices and returns the index of the selected choice.
:: /C <choices> - Specifies the list of choices to be created. Default list is "YN"
:: /M <message> - Specifies the message to be displayed before
::                the prompt. If not specified, the utility
::                displays only a prompt.
:: /N           - Hides the list of choices in the prompt.
::                The message before the prompt is displayed and the choices are still enabled.
choice /C:1234 /M "Please enter your selection: " /N

:: The first option listed by choice's /C option will return an errorlevel value of 1, the second 2, and so on
if %errorlevel% equ 1 (
	SET keePass="%USERPROFILE%\KeePass\KeePass-2.30\KeePass.exe"
	SET kdb="%USERPROFILE%\KeePass\PasswordDatabase\PasswordDatabase.kdbx"
	
	echo I'll start KeePass for You
	START "" %keePass% %kdb%
)

:: I've converted these to one-liners simply for personal preference.
:: You can keep these the way you had them if you put them inside of parentheses like with option 1.
if %errorlevel% equ 2 call "%USERPROFILE%\backup.bat"
if %errorlevel% equ 3 start "" "C:\Program Files (x86)\Mozilla Firefox\firefox.exe"
if %errorlevel% equ 4 exit /b

:: Calls this script again, simulating a goto :MENU
:: Personally, I'd stick with a label and a goto in this instance,
:: but this is how you could do it if you don't want to use goto at all
call %0

If you just want to convert your labels into subroutines, it's perfectly fine to keep the :MENU label and last goto.

@echo off

:MENU
cls
echo Welcome %USERNAME%

echo 1 - Start KeePass
echo 2 - Backup
echo 3 - FireFox
echo 4 - Exit

choice /C:1234 /M "Please enter your selection: " /N

:: Call acts like goto, except the script returns back up here when the code reaches :eof
if %errorlevel% equ 1 call :StarKeePass
if %errorlevel% equ 2 call :Backup
if %errorlevel% equ 3 call :FireFox
if %errorlevel% equ 4 goto :EOF
GOTO MENU


:StarKeePass
echo SET keePass="%USERPROFILE%\KeePass\KeePass-2.30\KeePass.exe"
echo SET kdb="%USERPROFILE%\KeePass\PasswordDatabase\PasswordDatabase.kdbx"

echo I'll start KeePass for You
START "" %keePass% %kdb% 

GOTO :EOF

:Backup
SET backup="%USERPROFILE%\backup.bat"
call %backup%

GOTO :EOF

:FireFox
cd "C:\Program Files (x86)\Mozilla Firefox\"
start firefox.exe

GOTO :EOF

Nexusfactor

Member

Registered: 15 Dec 2015

Posts: 8

Re: Avoiding Spaghetti Code

Thanks for the clarification and code.

foxidrive

Member

Registered: 04 Apr 2013

Posts: 339

Re: Avoiding Spaghetti Code

This was asked here too www.computerhope.com/forum/index.php/topic,153328.0.html

@ECHO OFF
CLS
:MENU
echo Welcome %USERNAME%

echo 1 - Start KeePass
echo 2 - Backup
echo 3 - FireFox
echo 4 - Exit
echo(

SET "keePass=%USERPROFILE%\KeePass\KeePass-2.30\KeePass.exe"
SET "kdb=%USERPROFILE%\KeePass\PasswordDatabase\PasswordDatabase.kdbx"
SET "backup=%USERPROFILE%\backup.bat"
set "m="
SET /P "M=Please Enter Selection, then Press Enter: "
IF "%M%"=="1" (echo I'll start KeePass for You & START "" "%keePass%" "%kdb%")
IF "%M%"=="2" (call "%backup%")
IF "%M%"=="3" (start "" /d "C:\Program Files (x86)\Mozilla Firefox\" firefox.exe)
IF "%M%"=="4" (GOTO :EOF)
GOTO MENU

Shadow Thief

Member

Registered: 12 Jul 2012

Posts: 205

Re: Avoiding Spaghetti Code

And here (http://stackoverflow.com/questions/3431 … hetti-code) but I already posted my solution there as well.

Nexusfactor

Member

Registered: 15 Dec 2015

Posts: 8

Re: Avoiding Spaghetti Code

Many thanks to Foxidrive and Shadow Thief.

I posted it in more than one location, because I didn't expect a response so fast. I don't mind using SO, it's just that some of the people responding can be rude. I looked for alternative sites/forums.

Again, thanks for the help.

Shadow Thief

Member

Registered: 12 Jul 2012

Posts: 205

Re: Avoiding Spaghetti Code

I posted it in more than one location, because I didn't expect a response so fast. I don't mind using SO, it's just that some of the people responding can be rude.

Can't argue with that, especially since I am one of those people. I'm way nicer here and on DosTips, and I'll actually just flat-out give you code instead of telling you to try it yourself.