Lucas
Lucas

Reputation: 648

If statement ( was unexpected at this time

I have a bundled if statement that checks whether java is installed and whether or not i'm at college or at home it is part of a larger piece of code but doesn't work on its own either any help would be much appreciated

Triple Checked i have the right number of brackets and equals signs and making sure no obvious mistakes were made so i'm confused as to what has gone wrong

if exist 'C:\Temporary Workspace' (set installtype=College)
where java >nul 2>nul
pause
if %errorlevel%==1 (
    if %installtype%==College (
    goto :college
    ) else (
        set /P javaboolean=Java was not found is java installed? (Y/N)
        if %javaboolean%==Y (
            echo Please enter the path to java.exe
            set /P javalocation=e.g. C:\Program Files\Common Files\Oracle\Java\javapath\java.exe
        ) 
    )
)
pause
exit
:college
echo success for college detection
pause

The error message and run time:


D:\Batch Testing>echo Checking Location...
Checking Location...

D:\Batch Testing>if exist 'C:\Temporary Workspace' (set installtype=College)

D:\Batch Testing>where java  1>nul 2>nul

D:\Batch Testing>pause
Press any key to continue . . .
( was unexpected at this time.


I expect the output to be 'success for college detection' as the directory does exist

Upvotes: 1

Views: 1564

Answers (2)

PA.
PA.

Reputation: 29339

you need to change your ifs a little bit

the first could be changed to

if exist "C:\Temporary Workspace" set installtype=College

note that the single apostrophe is not a valid quotation mark in windows cmd. And you can remove your parentheses because they are not needed.

the second could be written as

if errorlevel 1 (

because errorlevel is not %errorlevel% (for a detailed explanation, read https://devblogs.microsoft.com/oldnewthing/20080926-00/?p=20743 )

and the last could be

if .%installtype%==.College (

because when installtype is blank the if instruction makes no sense and produces syntax error; inserting the two . avoids it.

... and, as a bonus, you might want to consider to use if /i to ignore case when comparing, so College, college, COLLEGE or even cOllegE would match.

Upvotes: 3

aschipfl
aschipfl

Reputation: 34909

Well, there are several issues in your code. The line

if exist 'C:\Temporary Workspace' (set installtype=College)

will never set the variable installtype, because this checks whether 'C:\Temporary exists, which does not, since the apostrope ' is just an ordinary character for the Windows Command Prompt cmd. To make it work you must use quotation marks ":

if exist "C:\Temporary Workspace\" (set "installtype=College")

The trailing \ lets the condition check for existence of a directory called Temporary Workspace but not for a file with that name.

Note that I used the quoted set syntax here, which is generally recommended as it protects special characters.


The condition

    if %installtype%==College (

will return a syntax error, because the variable installtype has not been set before (due to the wrong syntax you used). To avoid trouble with empty variables, quote both comparison expressions:

    if "%installtype%"=="College" (

This is also described in this thread.


The command

        set /P javaboolean=Java was not found is java installed? (Y/N)

will fail when in a parenthesised block of code, because the ) will unintentionally be recognised by cmd and close the block at an undesired position. Again use quotation to solve that:

        set /P javaboolean="Java was not found is java installed? (Y/N)"

or:

        set /P "javaboolean=Java was not found is java installed? (Y/N)"

However, the condition

        if %javaboolean%==Y (

will still fail, because the queried variable javaboolean is set in the same block of code, so the returned value is the one present when the entire block is parsed, which is an empty string most probably. To overcome this, you must apply delayed variable expansion.

Put this somewhere before to enable delayed expansion:

        setlocal EnableDelayedExpansion

Then replace the %-signs by exclamation marks ! (as you can see I again applied quotation here):

        if "!javaboolean!"=="Y" (

Then put this somewhere after to end the environment localisation introduced by setlocal:

        endlocal

Regard that all environment changes (variables, current directories) after setlocal are lost after endlocal and the state before setlocal becomes restored.

A similar problem is also described in this question and solved by this answer.


Instead of set /P just for a Yes/No decision consider to use the choice command, which does not allow to enter anything else:

rem /* The default choices are `Y` and `N`;
rem    `choice` sets `ErrorLevel` to `1` when pressing `Y` and to `2` when pressing `N`: */
choice /M "Java was not found is java installed"
rem // The following means: if ErrorLevel >= 1 and if ErrorLevel < 2:
if ErrorLevel 1 if not ErrorLevel 2 (echo Yes has been chosen.)

Instead of

exit

you should use

exit /B

to only terminate the batch script but keep the hosting cmd instance alive. This is particularly helpful when you execute the batch file from a Command Prompt window rather than by double-clicking its icon in Explorer as you can view the console output then.


Instead of the string comparison

if %errorlevel%==1 (

you could also do true numeric comparison like

if %errorlevel% equ 1 (

or, if you are fine with the condition to be true if ErrorLevel is also greater than one,

if ErrorLevel 1 (

Alternatively, you could even use condition execution operators, which query the exit code:

where java > nul 2>&1 && (
    rem // The code here is executed when `where` succeeds.
) || (
    rem // The code here is executed when `where` fails.
)

So finally, here is the fixed script:

  1. Code based on yours:

    if exist "C:\Temporary Workspace\" (set "installtype=College")
    where java > nul 2>&1
    pause
    if ErrorLevel 1 (
        if "%installtype%"=="College" (
            goto :college
        ) else (
            set /P javaboolean="Java was not found is java installed? (Y/N) "
            setlocal EnableDelayedExpansion
            if "!javaboolean!"=="Y" (
                endlocal
                echo Please enter the path to java.exe
                set /P javalocation="e.g. C:\Program Files\Common Files\Oracle\Java\javapath\java.exe "
            ) else endlocal
        )
    )
    pause
    exit /B
    
    :college
    echo success for college detection
    pause
    
  2. Variant using conditional execution (||) and the choice command:

    if exist "C:\Temporary Workspace\" (set "installtype=College")
    where java > nul 2>&1 || (
        if "%installtype%"=="College" (
            goto :college
        ) else (
            choice /M "Java was not found is java installed"
            if ErrorLevel 1 if not ErrorLevel 2 (
                echo Please enter the path to java.exe
                set /P javalocation="e.g. C:\Program Files\Common Files\Oracle\Java\javapath\java.exe "
            )
        )
    )
    pause
    exit /B
    
    :college
    echo success for college detection
    pause
    

Upvotes: 0

Related Questions