debugMinor

Is there a better way to optimize these conditionals?

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

theseconditionalswaybetteroptimizethere

Problem

Public Function MyProjectFuncion(firstObject As Object,
                                 myEnumeration As Enumeration,
                                 theString As String,
                                 someCollection As List(of Object),
                                 anotherObject As Object) _
                            As String

    If IsNothing(firstObject) Then
        Throw New Exception("No data was received in 'firstObject'")
    Else
        If IsNothing(anotherObject) Then
            Throw New Exception("No data was received in 'anotherObject'")
        Else
            If IsNothing(myEnumeration) OrElse myEnumeration = 0 Then
                Throw New Exception("No data was received in 'myEnumeration'")
            Else
                If IsNothing(theString) OrElse theString = String.Empty Then
                    Throw New Exception("No data was received in 'theString'")
                Else
                    If IsNothing(someCollection) OrElse someCollection.Count = 0 Then
                        Throw New Exception("No data was received in 'someCollection'")
                    End If
                End If
            End If
        End If
    End If

    Return "All the variables have value"
End Function

Solution

You're throwing System.Exception - that's bad, you'll want to throw a meaningful exception. See this answer for more details.

The main problem with this code, is that you're using exceptions to control the execution flow of your program, which is very expensive - I assume you're going to be catching these exceptions somewhere down the call stack.

If a is Nothing, then accessing it will throw an NullReferenceException. Same for b, c, d and e (say, you also have f in there?).

Let it blow up with whatever exception you get - and catch it. Use exceptions for exceptional cases, and avoid throwing them altogether if you can.

"Exceptions are expensive"

"Throwing and catching exceptions is expensive", you'll read [almost] everywhere. There's truth in that, Jon Skeet has a great article here that correlates performance to the depth of the call stack. The reason for this, is because the thread stops at the throw instruction, and then the exception's StackTrace gets populated - the call stack gets unwinded and the thread walks it until it reaches a catch block. The deeper the stack, the longer it takes.

But the problem isn't about performance - it's about efficiency. Exceptions are not a way to control your execution flow. Regardless of whether or not you care about wasting cycles and populating a stack trace, regardless of how long that takes for the runtime to complete, the point and the matter is, there are ways to make the intent much clearer to whoever is maintaining this code, than by throwing an exception.

The goal is to validate a bunch of objects, and return a message that informs which object is in an invalid state.

By throwing an exception, the first invalid object exits the function and you can't know if other ones were invalid.

By nesting the conditions and returning a String, the last invalid object gets its status reported... if you're lucky - I mean, when anotherObject isn't null, nothing else gets checked and you can't report that theString was NullOrEmpty.

How about returning a flag enum that contains all validation errors? (not sure of VB syntax here, my background is C#):

[Flags]
Public Enum MyValidationError
    None = 0
    FirstObjectIsInvalid = 1
    MyEnumerationIsInvalid = 2
    TheStringIsInvalid = 4
    SomeCollectionIsInvalid = 8
    AnotherObjectIsInvalid = 16
End Enum

Then the function can return a MyValidationError value:

Public Function MyProjectFunction(...) As MyValidationError

    Dim result As MyValidationError
    If IsNothing(firstObject) Then result += MyValidationError.FirstObjectIsInvalid
    If IsNothing(anotherObject) Then result += MyValidationError.AnotherObjectIsInvalid
    If MyEnumeration = 0 Then result += MyValidationError.MyEnumerationIsInvalid 'an enum type shouldn't ever be 'Nothing'...
    If String.IsNullOrEmpty(theString) Then result += MyValidationError.TheStringIsInvalid
    If IsNothing(SomeCollection) OrElse SomeCollection.Count = 0 Then result += MyValidationError.SomeCollectionIsInvalid

    Return result

End Function

Notice all parameters are validated every time now, and the result will contain everything that went wrong. Then another method can use bitwise AND operations on the result and determine the message(s) to log/display, all without throwing an exception, and without nesting anything.

Code Snippets

[Flags]
Public Enum MyValidationError
    None = 0
    FirstObjectIsInvalid = 1
    MyEnumerationIsInvalid = 2
    TheStringIsInvalid = 4
    SomeCollectionIsInvalid = 8
    AnotherObjectIsInvalid = 16
End Enum

Public Function MyProjectFunction(...) As MyValidationError

    Dim result As MyValidationError
    If IsNothing(firstObject) Then result += MyValidationError.FirstObjectIsInvalid
    If IsNothing(anotherObject) Then result += MyValidationError.AnotherObjectIsInvalid
    If MyEnumeration = 0 Then result += MyValidationError.MyEnumerationIsInvalid 'an enum type shouldn't ever be 'Nothing'...
    If String.IsNullOrEmpty(theString) Then result += MyValidationError.TheStringIsInvalid
    If IsNothing(SomeCollection) OrElse SomeCollection.Count = 0 Then result += MyValidationError.SomeCollectionIsInvalid

    Return result

End Function

Context

StackExchange Code Review Q#46957, answer score: 8

Revisions (0)

No revisions yet.