Is there any way I could improve this batch script? In terms of formatting, syntax, etc

Question

SO keeps telling me "Please add some context to explain the code sections (or check that you have no incorrectly formatted all of your questions as code). Why does it keep telling me this? I do have comments within the code which explain what each section does. After adding this paragraph, it seems to have allowed me to post.

REM Runs scva.exe if it is not running; kills process after dosbox.exe is terminated

tasklist /FI "IMAGENAME eq scva.exe" 2>NUL | find /I /N "scva.exe">NUL
if "%ERRORLEVEL%"=="0" (
    goto LoopStart
) else (
    cd "%~dp0../"
    start /min "" "scva.exe"
    goto FirstCheck
)

:FirstCheck
REM Waits until dosbox.exe is started before continuing, so the script doesn't terminate prematurely

tasklist /FI "IMAGENAME eq dosbox.exe" 2>NUL | find /I /N "dosbox.exe">NUL
if "%ERRORLEVEL%"=="0" (
    goto LoopStart
) else (
    goto FirstCheck
)

:LoopStart
REM Exits script if scva.exe is manually terminated, otherwise, goes to CheckDOSBox

tasklist /FI "IMAGENAME eq scva.exe" 2>NUL | find /I /N "scva.exe">NUL
if "%ERRORLEVEL%"=="0" (
    goto CheckDOSBox
) else (
    exit
)

:CheckDOSBox
REM Goes to LoopStart while dosbox.exe is running, otherwise, goes to Kill

tasklist /FI "IMAGENAME eq dosbox.exe" 2>NUL | find /I /N "dosbox.exe">NUL
if "%ERRORLEVEL%"=="0" (
    goto LoopStart
) else (
    goto Kill
)

:Kill
REM Sends command to terminate scva.exe while scva.exe is running, otherwise, exits script

tasklist /FI "IMAGENAME eq scva.exe" 2>NUL | find /I /N "scva.exe">NUL
if "%ERRORLEVEL%"=="0" (
    taskkill /IM scva.exe
    goto Kill
)
exit

This question maybe suits better on https://codereview.stackexchange.com/... — aschipfl, Jun 07 '17 at 00:26
Did I not propose an alternative [in the last question you asked when intending to convert the code to VBS](https://stackoverflow.com/a/44391410/6738015)? — Compo, Jun 07 '17 at 00:27
Wow, somehow I completely missed that. That looks a lot cleaner. — Lucas, Jun 08 '17 at 02:34

score 1 · Answer 1 · answered Jun 07 '17 at 00:10

Using conditionall execution on fail || or success && you can avoid all the if's.
if the label you want to jump to is the next statement, it can be omitted - program flow will reach it nevertheless.
I'd insert some timeout /t 1 or ping -n 1 localhost to have a 1 second delay.

REM Runs scva.exe if it is not running; kills process after dosbox.exe is terminated

tasklist /FI "IMAGENAME eq scva.exe" 2>NUL | find /I /N "scva.exe">NUL && goto LoopStart
cd "%~dp0../"
start /min "" "scva.exe"

:FirstCheck
REM Waits until dosbox.exe is started before continuing, so the script doesn't terminate prematurely

tasklist /FI "IMAGENAME eq dosbox.exe" 2>NUL | find /I /N "dosbox.exe">NUL || goto FirstCheck

:LoopStart
REM Exits script if scva.exe is manually terminated, otherwise, goes to CheckDOSBox

tasklist /FI "IMAGENAME eq scva.exe" 2>NUL | find /I /N "scva.exe">NUL || exit

:CheckDOSBox
REM Goes to LoopStart while dosbox.exe is running, otherwise, goes to Kill

tasklist /FI "IMAGENAME eq dosbox.exe" 2>NUL | find /I /N "dosbox.exe">NUL && goto LoopStart

:Kill
REM Sends command to terminate scva.exe while scva.exe is running, otherwise, exits script

tasklist /FI "IMAGENAME eq scva.exe" 2>NUL | find /I /N "scva.exe">NUL || exit
taskkill /IM scva.exe
goto Kill

Is there any way I could improve this batch script? In terms of formatting, syntax, etc

1 Answers1