How to avoid this excessive number of if/elseif statements?

SlyraksL · July 13

Greetings, I'm working on my first mod that add a dialogue option to the Braig question "Do you have any family? Anyone waiting for you on the outside?" in Cidhna Mine wich allows the player to mention their adopted children (if there is any adopted child)

I created a script in the Son/Daughter adoption confirmation dialogue that updates some quest stages, which will be used by the Conditional Functions of the Braig question, but i don't know how to make a decent code structure to avoid this excessive number of IF's statements

Quest Property RelationshipAdoption Auto

Quest Property OnlyDaughtersQuestProperty Auto

Quest Property OnlySonsQuestProperty Auto

Quest Property SameAmountQuestProperty Auto

Quest Property MoreDaughtersQuestProperty Auto

Quest Property MoreSonsQuestProperty Auto

GlobalVariable Property DaughtersVariableProperty Auto

GlobalVariable Property SonsVariableProperty Auto

; updates the number of daughters when completing an adoption

int DaughtersCount = DaughtersVariableProperty.GetValueInt()

DaughtersVariableProperty.SetValueInt(DaughtersCount + 1)

; returns the number of daughters to be used by the conditions

int DaughtersCount2 = DaughtersVariableProperty.GetValueInt()

; returns the number of sons to be used by the conditions

int SonsCount = SonsVariableProperty.GetValueInt()

; invalidates the ONLY SONS quest when adopting a daughter

OnlySonsQuestProperty.SetStage(100)

; ONLY DAUGHTERS conditions

if DaughtersCount2 == 1

OnlyDaughtersQuestProperty.SetStage(10)

Elseif DaughtersCount2 == 2

OnlyDaughtersQuestProperty.SetStage(20)

Elseif DaughtersCount2 == 3

OnlyDaughtersQuestProperty.SetStage(30)

Elseif DaughtersCount2 == 4

OnlyDaughtersQuestProperty.SetStage(40)

Elseif DaughtersCount2 == 5

OnlyDaughtersQuestProperty.SetStage(50)

Elseif DaughtersCount2 == 6

OnlyDaughtersQuestProperty.SetStage(60)

endif

; SAME AMOUNT of sons and daughters conditions

While (DaughtersCount2 == SonsCount)

if DaughtersCount2 == 1

SameAmountQuestProperty.SetStage(11)

Elseif DaughtersCount2 == 2

SameAmountQuestProperty.SetStage(22)

MoreSonsQuestProperty.SetStage(22)

MoreDaughtersQuestProperty.SetStage(22)

Elseif DaughtersCount2 == 3

SameAmountQuestProperty.SetStage(33)

MoreSonsQuestProperty.SetStage(33)

MoreDaughtersQuestProperty.SetStage(33)

endif

EndWhile

; MORE DAUGHTERS than sons conditions

if (DaughtersCount2 == 2) && (SonsCount == 1)

MoreDaughtersQuestProperty.SetStage(21)

SameAmountQuestProperty.SetStage(15)

MoreSonsQuestProperty.SetStage(12)

ElseIf (DaughtersCount2 == 3) && (SonsCount == 1)

MoreDaughtersQuestProperty.SetStage(31)

MoreSonsQuestProperty.SetStage(13)

ElseIf (DaughtersCount2 == 3) && (SonsCount == 2)

MoreDaughtersQuestProperty.SetStage(32)

SameAmountQuestProperty.SetStage(25)

MoreSonsQuestProperty.SetStage(23)

ElseIf (DaughtersCount2 == 4) && (SonsCount == 1)

MoreDaughtersQuestProperty.SetStage(41)

MoreSonsQuestProperty.SetStage(14)

ElseIf (DaughtersCount2 == 4) && (SonsCount == 2)

MoreDaughtersQuestProperty.SetStage(42)

MoreSonsQuestProperty.SetStage(24)

ElseIf (DaughtersCount2 == 5) && (SonsCount == 1)

MoreDaughtersQuestProperty.SetStage(51)

MoreSonsQuestProperty.SetStage(15)

endif

; MORE SONS conditions

if (SonsCount == 2) && (DaughtersCount2 == 1)

MoreSonsQuestProperty.SetStage(21)

SameAmountQuestProperty.SetStage(15)

MoreDaughtersQuestProperty.SetStage(12)

Elseif (SonsCount == 3) && (DaughtersCount2 == 1)

MoreSonsQuestProperty.SetStage(31)

MoreDaughtersQuestProperty.SetStage(13)

ElseIf (SonsCount == 3) && (DaughtersCount2 == 2)

MoreSonsQuestProperty.SetStage(32)

SameAmountQuestProperty.SetStage(25)

MoreDaughtersQuestProperty.SetStage(23)

ElseIf (SonsCount == 4) && (DaughtersCount2 == 1)

MoreSonsQuestProperty.SetStage(41)

MoreDaughtersQuestProperty.SetStage(14)

ElseIf (SonsCount == 4) && (DaughtersCount2 == 2)

MoreSonsQuestProperty.SetStage(42)

MoreDaughtersQuestProperty.SetStage(24)

ElseIf (SonsCount == 5) && (DaughtersCount2 == 1)

MoreSonsQuestProperty.SetStage(51)

MoreDaughtersQuestProperty.SetStage(15)

endif

Sphered · July 13

Quick one at a glance. You can use math for sections like your first one there. Below replaces all the if/else for that section

OnlyDaughtersQuestProperty.SetStage(DaughtersCount2  * 10)

Otherwise I'm sure others will chime in. That one just stood out

SlyraksL · July 13

18 minutes ago, Sphered said:
Quick one at a glance. You can use math for sections like your first one there. Below replaces all the if/else for that section
OnlyDaughtersQuestProperty.SetStage(DaughtersCount2  * 10)
Otherwise I'm sure others will chime in. That one just stood out

Thanks

xkkmEl · July 13

There are indeed a few "10 * a + b" math tricks you could use, but really you are looking at some complex and quirky logic. The code is gonna reflect that to some extend.

I note though that the use of "while" in your code is suspicious. Are you sure that works?

Making a subfunction to handle each "property" one at a time may provide with clearer logic and more concise code.

function __MoreDaughters()
	if (DaughtersCount2 == SonsCount) && (DaughtersCount2 == 2 || DaughtersCount2 == 3)
   	    MoreDaughtersQuestProperty.SetStage(11 * DaughtersCount2)
	EndIf
 
	if (DaughtersCount2 == 2) && (SonsCount == 1)
		MoreDaughtersQuestProperty.SetStage(10 * DaughtersCount2 + SonsCount)
	ElseIf (DaughtersCount2 == 3) && (SonsCount == 1 || SonsCount == 2)
		MoreDaughtersQuestProperty.SetStage(10 * DaughtersCount2 + SonsCount)
	ElseIf (DaughtersCount2 == 4) && (SonsCount == 1 || SonsCount == 2)
		MoreDaughtersQuestProperty.SetStage(10 * DaughtersCount2 + SonsCount)
	ElseIf (DaughtersCount2 == 5) && (SonsCount == 1)
		MoreDaughtersQuestProperty.SetStage(10 * DaughtersCount2 + SonsCount)
	endif
 
	if SonsCount >= 2 && SonsCount <= 5 && (DaughtersCount2 == 1)
		MoreDaughtersQuestProperty.SetStage(10 + SonsCount)
	ElseIf (SonsCount == 3 || SonsCount == 4) && (DaughtersCount2 == 2)
		MoreDaughtersQuestProperty.SetStage(20 + SonsCount)
	endif
endfunction

SlyraksL · July 14

1 hour ago, xkkmEl said:

There are indeed a few "10 * a + b" math tricks you could use, but really you are looking at some complex and quirky logic. The code is gonna reflect that to some extend.

I note though that the use of "while" in your code is suspicious. Are you sure that works?

Making a subfunction to handle each "property" one at a time may provide with clearer logic and more concise code.

function __MoreDaughters()
	if (DaughtersCount2 == SonsCount) && (DaughtersCount2 == 2 || DaughtersCount2 == 3)
   	    MoreDaughtersQuestProperty.SetStage(11 * DaughtersCount2)
	EndIf
 
	if (DaughtersCount2 == 2) && (SonsCount == 1)
		MoreDaughtersQuestProperty.SetStage(10 * DaughtersCount2 + SonsCount)
	ElseIf (DaughtersCount2 == 3) && (SonsCount == 1 || SonsCount == 2)
		MoreDaughtersQuestProperty.SetStage(10 * DaughtersCount2 + SonsCount)
	ElseIf (DaughtersCount2 == 4) && (SonsCount == 1 || SonsCount == 2)
		MoreDaughtersQuestProperty.SetStage(10 * DaughtersCount2 + SonsCount)
	ElseIf (DaughtersCount2 == 5) && (SonsCount == 1)
		MoreDaughtersQuestProperty.SetStage(10 * DaughtersCount2 + SonsCount)
	endif
 
	if SonsCount >= 2 && SonsCount <= 5 && (DaughtersCount2 == 1)
		MoreDaughtersQuestProperty.SetStage(10 + SonsCount)
	ElseIf (SonsCount == 3 || SonsCount == 4) && (DaughtersCount2 == 2)
		MoreDaughtersQuestProperty.SetStage(20 + SonsCount)
	endif
endfunction

Thanks for your attention! The use of subfunctions actually makes the code cleaner

The "While" seems to be working as I tested, but I used it just to practice a recently learned syntax, with no big plan in mind

Again, tysm

dylbill · July 14

I agree the while loop looks suspicious. When do DaughtersCount2 or SonsCount change? Because if those stay the same for a long time that while loop will continue to run. Probably better to use a RegisterForSingleUpdate loop if you're using it for polling instead of the while loop.

For things like this I prefer to use arrays. As an example I'll use the first condition set. You can definitely use *10 for that but it's just good for example purposes.

GlobalVariable Property DaughtersVariableProperty Auto

Quest Property OnlyDaughtersQuestProperty Auto

int[] Property DaughtersCount2Stages Auto

Event OnInit()

CreateArrays()

EndEvent

Function CreateArrays()

;you can also set this array in the creation kit directly

DaughtersCount2Stages = new int[7]

DaughtersCount2Stages[0] = 0 ;unused

DaughtersCount2Stages[1] = 10

DaughtersCount2Stages[2] = 20

DaughtersCount2Stages[3] = 30

DaughtersCount2Stages[4] = 40

DaughtersCount2Stages[5] = 50

DaughtersCount2Stages[6] = 60

EndFunction

;Edit: I guess you should check if DaughtersCount2 is in range first if you know it can be out of range.

Function SetOnlyDaughtersQuestStage()

int DaughtersCount2 = DaughtersVariableProperty.GetValueInt()

if DaughtersCount2 >= 1 && DaughtersCount2 <= 6

OnlyDaughtersQuestProperty.SetStage(DaughtersCount2Stages[DaughtersCount2])

Endif

EndFunction

PeterMartyr · July 14

literating or recursive code are the only options, plus refactoring code if get uses a lot to reduce repetition..

but in your case even a switch would not work and a sometimes a ElseIf is the only option.... has well

https://dasha.ai/en-us/blog/javascript-if-else-or-switch-case

EDIT not that papyrus supports a switch, but if if did, it would not work in your example, so if you cannot loop it. (I do not think recursion is an option for you either) sometimes an ElseIf is the best option available

Plus now you know the first option when optimising an ElseIf is a Switch, not that papyrus supports it but keep in mind with other languages

I would try a combination of iterative and refactoring, if possible, to me it looks superficially with the limited knowledge I have like an ElseIf is a good option here

plus I agree with @Sphered too, the others are too chaotic, but this stood out to me

While (DaughtersCount2 == SonsCount)
    if DaughtersCount2 == 1
        SameAmountQuestProperty.SetStage(11)
    Elseif DaughtersCount2 == 2
        SameAmountQuestProperty.SetStage(22)
        MoreSonsQuestProperty.SetStage(22)
        MoreDaughtersQuestProperty.SetStage(22)
    Elseif DaughtersCount2 == 3
        SameAmountQuestProperty.SetStage(33)
        MoreSonsQuestProperty.SetStage(33)
        MoreDaughtersQuestProperty.SetStage(33)
    endif
EndWhile

did you mean to do an IF?

If (DaughtersCount2 == SonsCount)
    if DaughtersCount2 == 1
        SameAmountQuestProperty.SetStage(11)
    Elseif DaughtersCount2 == 2
        SameAmountQuestProperty.SetStage(22)
        MoreSonsQuestProperty.SetStage(22)
        MoreDaughtersQuestProperty.SetStage(22)
    Elseif DaughtersCount2 == 3
        SameAmountQuestProperty.SetStage(33)
        MoreSonsQuestProperty.SetStage(33)
        MoreDaughtersQuestProperty.SetStage(33)
    endif
EndIf

are you sure it working? cos three of us think it is bad, only I am gonna spelled it out.. how do you break the loop if it is true? You have an infinite loop mate, plus the logic of using a loop their has no benefits as well so it has two problems

While (DaughtersCount2 == SonsCount)

SlyraksL · July 14

5 hours ago, PeterMartyr said:
literating or recursive code are the only options, plus refactoring code if get uses a lot to reduce repetition..

but in your case even a switch would not work and a sometimes a ElseIf is the only option.... has well

https://dasha.ai/en-us/blog/javascript-if-else-or-switch-case

EDIT not that papyrus supports a switch, but if if did, it would not work in your example, so if you cannot loop it. (I do not think recursion is an option for you either) sometimes an ElseIf is the best option available

Plus now you know the first option when optimising an ElseIf is a Switch, not that papyrus supports it but keep in mind with other languages

I would try a combination of iterative and refactoring, if possible, to me it looks superficially with the limited knowledge I have like an ElseIf is a good option here

plus I agree with @Sphered too, the others are too chaotic, but this stood out to me
While (DaughtersCount2 == SonsCount)
    if DaughtersCount2 == 1
        SameAmountQuestProperty.SetStage(11)
    Elseif DaughtersCount2 == 2
        SameAmountQuestProperty.SetStage(22)
        MoreSonsQuestProperty.SetStage(22)
        MoreDaughtersQuestProperty.SetStage(22)
    Elseif DaughtersCount2 == 3
        SameAmountQuestProperty.SetStage(33)
        MoreSonsQuestProperty.SetStage(33)
        MoreDaughtersQuestProperty.SetStage(33)
    endif
EndWhile
did you mean to do an IF?
If (DaughtersCount2 == SonsCount)
    if DaughtersCount2 == 1
        SameAmountQuestProperty.SetStage(11)
    Elseif DaughtersCount2 == 2
        SameAmountQuestProperty.SetStage(22)
        MoreSonsQuestProperty.SetStage(22)
        MoreDaughtersQuestProperty.SetStage(22)
    Elseif DaughtersCount2 == 3
        SameAmountQuestProperty.SetStage(33)
        MoreSonsQuestProperty.SetStage(33)
        MoreDaughtersQuestProperty.SetStage(33)
    endif
EndIf
are you sure it working? cos three of us think it is bad, only I am gonna spelled it out.. how do you break the loop if it is true? You have an infinite loop mate, plus the logic of using a loop their has no benefits as well so it has two problems
While (DaughtersCount2 == SonsCount)

Yea, I did more tests and this While loop is indeed a bad idea

Skyrim

How to avoid this excessive number of if/elseif statements?

Recommended Posts

SlyraksL

Link to comment

Share on other sites

Sphered

Link to comment

Share on other sites

SlyraksL

Link to comment

Share on other sites

xkkmEl

Link to comment

Share on other sites

SlyraksL

Link to comment

Share on other sites

dylbill

Link to comment

Share on other sites

PeterMartyr

Link to comment

Share on other sites

SlyraksL

Link to comment

Share on other sites

Recently Browsing 0 members

Browse

Activity

My Activity Streams

Explore