batch file display prompt not correct

Question

There are two issues with the batch below. The first is, when batch file when opened prompts the user with a "y/n" question:

Question 1

Has the check been done

If the answer to this is "y" then another "y/n" question is displayed

Question 2

Do you want to send the DOSE report

If the answer to question 1 is "n" the check function is called and another question is displayed. However, currently the line in bold is being displayed then it is going to the second part (the goodbye function). What am I doing wrong? Thank you :).

Current batch

@ECHO OFF
:: ask user 
 :choice
 set /P c=Has the check been done [y/n]
 if /i %c%==y (
 set /P c=Do you want to send the DOSE report[y/n]?
 ) else (
 if /i %c%==n goto check
 )
 if /i %c%==y ( 
"L:\NGS\HLA LAB\total quality management\QC & QA\DOSE reports\DOSE reporting form.xlsm"
 ) else (
 if /i %c%==n goto goodbye
 )
 :check
 set /P c=Do you want to perform the check [y/n]
 if /i %c%==y (
 echo "perform check and hit enter when complete" 
 pause goto choice
 )    else (
 if /i %c%==n goto goodbye
 :: count loop
 set var1=0
 :loop
 set /a var1=%var1%+1

 echo %var1%

 if %var1% EQU 1 (
 goto end
 ) else (
 goto loop
 )
 :end
 echo "the DOSE report has already been sent by %USERNAME% on %DATE% at   %TIME%"
 :goodbye
 echo "goodbye"
 TIMEOUT 2 /nobreak
 exit

Answer 1

To answer your question directly... well, I'm not sure what you're asking to be honest. I'm having trouble deciphering "I am trying to have the below batch file increment the counter %var1 only if the "y" and not able to get the syntax correct."

I don't see any outright syntax errors, but you do have quite a few logic issues with your script. Indeed, the value of %var1% is being incremented. Put an echo Var1 incremented: %var1% after the set /a var1+=1 and see for yourself. To fix your script, you need to address what statements get executed under what conditions.

1. Use setlocal. It's just good practice. Using setlocal helps you avoid contaminating the outer console thread with variables that are only relevant to the runtime of this script.

2. Don't trust that the user will always answer "y" or "n". What happens if they answer "w"? As your script is written, it will proceed to sections you probably didn't intend. I have two suggestions for this.

   a. Don't explicitly check for an answer of "n". If it's not "y", assume it's "n". If that is not appropriate, then have a final else goto label for whatever label immediately precedes the conditional.

   b. Alternatively, instead of set /P, consider using choice. Example:

   choice /c YN /m "Do you want to perform the check? "
   
   if errorlevel 2 (
       rem // user chose "N"
   ) else (
       rem // user chose "Y"
   )
   

3. In your :choice section of code, what is the difference between an answer of Y then N, versus an answer of N? As written, there's no difference. Either way, the script proceeds to :check. Examine your logic here.

4. Instead of exit, consider using either exit /b or goto :EOF. When running your script from a cmd prompt (as opposed to double-clicking it), you should avoid exiting the parent thread.

5. For goodness sake, indent your code and add some line breaks! That big blob of left-justified commands has no flow, no rhythm. It's tedious to read and troubleshoot.

Before:

@ECHO OFF
:: ask user 
 :choice
 set /P c=Has the check been done [y/n]
 if /i %c%==y (
 set /P c=Do you want to send the DOSE report[y/n]?
 ) else (
 if /i %c%==n goto check
 )
 :check
 set /P c=Do you want to perform the check [y/n]
 if /i %c%==y (
 set /P c=please complete the check and click enter
 goto file
 ) else (
 if /i %c%==n goto goodbye
 )

It's like a slab, difficult to see at a glance which statements are ancestors and which are descendents or where the section breaks occur.

After:

@ECHO OFF
setlocal

:choice // ask user
choice /c YN /m "Has the check been done? "
if not errorlevel 2 (
    choice /c YN /m "Do you want to send the DOSE report? "
    if errorlevel 2 (
        goto goodbye
    )
)

:check
choice /c YN /m "Do you want to perform the check? "
if errorlevel 2 goto goodbye

set /P "c=Please complete the check and click enter: "

See? It's much more pleasant to read that way. By the way, you might've noticed that I combined the "ask user" comment with the :choice label. That's often done when defining functions to explain expected parameters, or just explain what's going on. You don't really need the double slashes. It just makes Stack Overflow's Ted Turner Technicolor parser demonstrate that that's a comment.

Answer 2

First off, the if-else statement in :choice is missing its closing )

Heres an important note on user input, never trust it. Try putting something other than y/n in the first

:choice
set /p c=Has the check been done [y/n]
 if /i %c%==y (
   set /p c=Do you want to send the DOSE report[y/n]?
  ) else (
     if /i %c%==n goto check
)

If you input something invalid into the first if-else , say somebody tries typing no instead of n, it will fail to return them to :choice as it only checks for n or y

And end up running through script. In your case it fails the if statements before :check and starts :check's proccess, but in check the same issue arises, and it will run through to ::count loop and the following commands where it can mess up your data.

After each if-else statement, its VERY safe practice to add a default action, such as;

:choice
set /p c=Has the check been done [y/n]
 if /i %c%==y (
   set /p c=Do you want to send the DOSE report[y/n]?
  ) else (
    if /i %c%==n goto check
)

:[ If both if statements are false, it will reach this line: ]
cls
echo Error: "%c%" is not y/n.
pause
goto :choice

Another thing to note, if nothing is input, errors will occur. You can fix this by checking if the variable is defined right after set /p c=Has the check been done [y/n] using:

 if not defined c (
   cls
   echo Error: Input cannot be empty!
   pause
   goto :choice
)

So a proper way to do the first check would be:

:choice
set /p c=Has the check been done [y/n]
:[Empty check ]
if not defined c cls & echo Error: Input cannot be empty! & pause & goto :choice
 if /i "%c%==y" (
   set /p c=Do you want to send the DOSE report[y/n]?
  ) else (
    if /i "%c%==n" goto check
)

:[ Invalid input ]
cls & echo Error: "%c%" is not y/n. & pause & goto :choice