deeply nested if..then..else pattern question

  • Thread starter Thread starter ECathell
  • Start date Start date
E

ECathell

I had read an article at one time that suggested a pattern to get around deeply nested if..then..else hell...

Can anyone point me in that direction?

select case statements wont work for me in this instance
 
Eric,
Where you thinking of the "Replace Nested Conditional with Guard Clauses"
refactoring?

http://www.refactoring.com/catalog/replaceNestedConditionalWithGuardClauses.html


Can you post a sample of your nested ifs? Maybe then I or someone else on
the group could offer suggestions on how to get around them...

--
Hope this helps
Jay [MVP - Outlook]
T.S. Bradley - http://www.tsbradley.net

I had read an article at one time that suggested a pattern to get around
deeply nested if..then..else hell...

Can anyone point me in that direction?

select case statements wont work for me in this instance
 
Thanks for the response!
Here is the code after I worked on it some. I had had the 2 ifs as 4 ifs.
Before that it was even worse(as I learn my code gets neater..hehe.) I would
still like to break this down in a way that I could return a more
informative error message. I may eventually break it out into custom
exceptions. But I would still need 4 if...thens instead of 2...

Me.m_Pallet = loadDatatable(m_palletid, m_Pallet)
If Me.m_Pallet.Rows.Count > 0 Then
'isArea>79
If Me.checkArea(m_Pallet) = True And Me.checkRemoved(m_Pallet) =
True Then
'Is the Pallet Issued to Production?
If Me.checkIssued(m_Pallet) = False And Me.checkPicked(m_palletid)
= False Then
m_area = resetArea(m_area)
resetPallet(m_palletid, m_area)
m_fixed = True
End If
Else
MessageBox.Show("Error With Pallet.Cannot run Reconcile Fix")
m_fixed = False
End If
End If
 
ECathell,
As I suggested, rather then nest the ifs, I would use guard clauses,
something like:

Private Sub Checks()
m_pallet = LoadDataTable(m_palletid, m_pallet)

If Not (Me.m_Pallet.Rows.Count > 0) Then
Throw New InvalidOperationException("Pallet is empty")
End If

If Not (Me.checkArea(m_Pallet) = True And _
Me.checkRemoved(m_Pallet) = True) Then
Throw New InvalidOperationException("isArea>79")
End If

If Not (Me.checkIssued(m_Pallet) = False And _
Me.checkPicked(m_palletid) = False) Then
Throw New InvalidOperationException("'Is the Pallet Issued to
Production?")
End If

m_area = resetArea(m_area)
resetPallet(m_palletid, m_area)

End Sub

Public Sub Main()

Try
Checks()
Catch ex As Exception
MessageBox.Show(ex.Message)
End Try

End Sub

Each "guard clause" is its own If statement, if the statement fails it
throws an exception, you can of course use more descriptive Exception and
message.

Generally rather then use "If Not" as above, I would invert the condition:

If Me.m_Pallet.Rows.Count <= 0 Then
Throw New InvalidOperationException("Pallet is empty")
End If

I kept your conditions "as is", so you could see what I did.

--
Hope this helps
Jay [MVP - Outlook]
T.S. Bradley - http://www.tsbradley.net


| Thanks for the response!
| Here is the code after I worked on it some. I had had the 2 ifs as 4 ifs.
| Before that it was even worse(as I learn my code gets neater..hehe.) I
would
| still like to break this down in a way that I could return a more
| informative error message. I may eventually break it out into custom
| exceptions. But I would still need 4 if...thens instead of 2...
|
| Me.m_Pallet = loadDatatable(m_palletid, m_Pallet)
| If Me.m_Pallet.Rows.Count > 0 Then
| 'isArea>79
| If Me.checkArea(m_Pallet) = True And Me.checkRemoved(m_Pallet) =
| True Then
| 'Is the Pallet Issued to Production?
| If Me.checkIssued(m_Pallet) = False And
Me.checkPicked(m_palletid)
| = False Then
| m_area = resetArea(m_area)
| resetPallet(m_palletid, m_area)
| m_fixed = True
| End If
| Else
| MessageBox.Show("Error With Pallet.Cannot run Reconcile Fix")
| m_fixed = False
| End If
| End If
|
|
| --
| --Eric Cathell, MCSA
| message | > Eric,
| > Where you thinking of the "Replace Nested Conditional with Guard
Clauses"
| > refactoring?
| >
| >
http://www.refactoring.com/catalog/replaceNestedConditionalWithGuardClauses.html
| >
| >
| > Can you post a sample of your nested ifs? Maybe then I or someone else
on
| > the group could offer suggestions on how to get around them...
| >
| > --
| > Hope this helps
| > Jay [MVP - Outlook]
| > T.S. Bradley - http://www.tsbradley.net
| >
| > | > I had read an article at one time that suggested a pattern to get around
| > deeply nested if..then..else hell...
| >
| > Can anyone point me in that direction?
| >
| > select case statements wont work for me in this instance
| >
| > --
| > --Eric Cathell, MCSA
| >
| >
|
|
 
Thanks again for your help Jay. The exceptions were the way I was
leaning...Then I can be really specific...

thanks again...


--
--Eric Cathell, MCSA
Jay B. Harlow said:
ECathell,
As I suggested, rather then nest the ifs, I would use guard clauses,
something like:

Private Sub Checks()
m_pallet = LoadDataTable(m_palletid, m_pallet)

If Not (Me.m_Pallet.Rows.Count > 0) Then
Throw New InvalidOperationException("Pallet is empty")
End If

If Not (Me.checkArea(m_Pallet) = True And _
Me.checkRemoved(m_Pallet) = True) Then
Throw New InvalidOperationException("isArea>79")
End If

If Not (Me.checkIssued(m_Pallet) = False And _
Me.checkPicked(m_palletid) = False) Then
Throw New InvalidOperationException("'Is the Pallet Issued to
Production?")
End If

m_area = resetArea(m_area)
resetPallet(m_palletid, m_area)

End Sub

Public Sub Main()

Try
Checks()
Catch ex As Exception
MessageBox.Show(ex.Message)
End Try

End Sub

Each "guard clause" is its own If statement, if the statement fails it
throws an exception, you can of course use more descriptive Exception and
message.

Generally rather then use "If Not" as above, I would invert the condition:

If Me.m_Pallet.Rows.Count <= 0 Then
Throw New InvalidOperationException("Pallet is empty")
End If

I kept your conditions "as is", so you could see what I did.

--
Hope this helps
Jay [MVP - Outlook]
T.S. Bradley - http://www.tsbradley.net


| Thanks for the response!
| Here is the code after I worked on it some. I had had the 2 ifs as 4
ifs.
| Before that it was even worse(as I learn my code gets neater..hehe.) I
would
| still like to break this down in a way that I could return a more
| informative error message. I may eventually break it out into custom
| exceptions. But I would still need 4 if...thens instead of 2...
|
| Me.m_Pallet = loadDatatable(m_palletid, m_Pallet)
| If Me.m_Pallet.Rows.Count > 0 Then
| 'isArea>79
| If Me.checkArea(m_Pallet) = True And Me.checkRemoved(m_Pallet) =
| True Then
| 'Is the Pallet Issued to Production?
| If Me.checkIssued(m_Pallet) = False And
Me.checkPicked(m_palletid)
| = False Then
| m_area = resetArea(m_area)
| resetPallet(m_palletid, m_area)
| m_fixed = True
| End If
| Else
| MessageBox.Show("Error With Pallet.Cannot run Reconcile Fix")
| m_fixed = False
| End If
| End If
|
|
| --
| --Eric Cathell, MCSA
| message | > Eric,
| > Where you thinking of the "Replace Nested Conditional with Guard
Clauses"
| > refactoring?
| >
| >
http://www.refactoring.com/catalog/replaceNestedConditionalWithGuardClauses.html
| >
| >
| > Can you post a sample of your nested ifs? Maybe then I or someone else
on
| > the group could offer suggestions on how to get around them...
| >
| > --
| > Hope this helps
| > Jay [MVP - Outlook]
| > T.S. Bradley - http://www.tsbradley.net
| >
| > | > I had read an article at one time that suggested a pattern to get
around
| > deeply nested if..then..else hell...
| >
| > Can anyone point me in that direction?
| >
| > select case statements wont work for me in this instance
| >
| > --
| > --Eric Cathell, MCSA
| >
| >
|
|
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Back
Top