Best practices with a series of IF statements

[AHKxx]

I have a script that has been running well but has started giving me problems. The script is fairly simple, but it is lengthy, and a problem has developed somewhere inside it. I think it is probably a best practices issue, possibly combined with formatting issues, but I'd appreciate some insight.

This first snippet shows the start of the script including the hotkey.

Code: Select all

#m:: ; 
IfWinExist, Scrivener 
    SetKeydelay, 100 
InputBox, AHKronym, Enter AHKronym, type AHKronym  

Immediately following that, I've got a long series of Ifs, formatted as follows.

Code: Select all

if AHKronym = h1 ; scrivener heading 1
{		SetKeyDelay, 20
		Send !rra{Down}{Down 1}{Enter}
}  	
if AHKronym = h2 ; scrivener heading 2 
{		SetKeyDelay, 20
		Send !rra{Down 2}{Enter}
}  	
if AHKronym = h3 ; scrivener heading 3 
{		SetKeyDelay, 20
		Send !rra{Down 3}{Enter}
}  	
if AHKronym = h4 ; scrivener heading 4 
{		SetKeyDelay, 20
		Send !rra{Down 4}{Enter}
[b]		exit ; i have started adding this to terminate the whole thing after the desired If has run. [/b]
}  	
if AHKronym = vt 
{		SetKeyDelay, 20
		Send !v{Up}{Enter}
}  	
if AHKronym = fave ; open, then move and resize Scriv Favorites windows
		Send !dfm
{		SetKeyDelay, 100
		WinMove, DoMM,,1536,156,DEFAULT, 440 
}  	
if AHKronym = start ; goes to specified folder
{		Run C:\ProgramData\Microsoft\Windows\Start Menu\Programs
}  	

What I'm finding is that after the appropriate IF runs, the script will advance and execute certain other lines from other IF's, but not all of the lines that belong to that IF, and I can't tell why only those lines are being run.

For example, when I enter h1, h2, or h3 in the Input box, the corresponding If routine runs, but then it continues down to the WinMove statement that follow in the the IF below (next to last from bottom), where the WinTitle parameter matches the window that gets moved and resized (when it was just supposed to be a dialog box within that app that gets moved, and only when the appropriate If gets called.)

I've found that as in the h4 example, if I put in an Exit statement, the execution stops. I more or less understand why this is happening, but this has only started recently, and I've been adding to this script for almost a year, so there's something I'm not getting.

Should I be adding Exit at the end of all the If sequences? Or is there something I can do with the brackets within each of the Ifs, for example, putting the first one to the left of the If itself, making the entire thing self contained? Or should I be using Return in some way.

Thanks for any help on this. And apologies for the noobness of the question. I hope I've explained it clearly.

[maestrith]

My favorite style is

Code: Select all

if(value=1){
     MsgBox,1
}else{
     MsgBox,%value%
}

[FanaticGuru]

My preferred style:

Code: Select all

if (AHKronym = "h1") ; scrivener heading 1
{       
	SetKeyDelay, 20
	Send !rra{Down}{Down 1}{Enter}
}

Or this for nested conditionals:

Code: Select all

if (x=1)
{
	Send A
}
else
{
	Send NOT 1
}

; or another example

if (x=1)
{
	Send A
}
else if (x=2)
{
	Send B
}
else if (x=3)
{
	Send C
}
else
{
	Send NOT 1, 2 or 3
}

; or yet another example

if (x=1)
{
	Send A
	if (y=1)
	{
		Send YA
		Send Done
	}
}

It is more verbose but I like my opening and closing brackets to match up.

Your real problem though seems to be not putting "" around text that you are comparing.
AHKronym = h1 this is a comparison of two variables. h1 is considered a variable name.

Also () around the comparison is recommended. You don't always need them but you will run into problems that can easily be avoided. I always use them unless it is a single variable that is the conditional. if Toggle for example. Toggle being a variable name.

Also

Code: Select all

if AHKronym = fave ; open, then move and resize Scriv Favorites windows
        Send !dfm
{       SetKeyDelay, 100
        WinMove, DoMM,,1536,156,DEFAULT, 440 
}   

this is probably not what you want.
The Send !dfm is conditional on the if statement but the

Code: Select all

{       SetKeyDelay, 100
        WinMove, DoMM,,1536,156,DEFAULT, 440 
}   

is always going to occur as it is not conditional.
if (x = y) makes one "line" conditional. Only the very next line is conditional. Now, {} creates a block, every things in that block is then treated as the one next "line".

If your code is even coming close to working it is luck as there are some serious flaws.

FG

[AHKxx]

Thank you for these replies. They've been very helpful.

[guest3456]

since all you're doing is checking the different possible values for the same "AHKronym" variable, then youd want to use else if after the first comparison, so that the others won't constantly need to be checked as well