1

The following code iterates through a set of processors and returns their ids. Hardware exploration is hazardous and I want to make this function is as stable as possible.

That is:

  • If an exception was thrown whilst preparing the iteration, return a null reference; and
  • If an exception was thrown during an iteration, simply skip this iteration, silently.

Code:

    Public ReadOnly Iterator Property ProcessorIds As IEnumerable(Of String) Implements IHardwareIds.ProcessorIds
        Get

            Try

                Dim BiosQuery As ObjectQuery = New SelectQuery("Win32_Processor") 'https://learn.microsoft.com/en-us/windows/win32/cimwin32prov/win32-processor
                Dim HardwarePieces As New ManagementObjectSearcher(BiosQuery)

                For Each MyInfo As ManagementObject In HardwarePieces.Get
                    Try
                        Dim ProcessorId As String = MyInfo("ProcessorId").ToString
                        Yield ProcessorId
                    Catch ex As Exception
                    End Try
                Next

            Catch ex As Exception
                Return Nothing
            End Try

        End Get
    End Property

The current code does not compile because using return in an iterator is forbidden.

The best I could think of was to replace Return Nothing by Yield Nothing. I expect it should result in an enumerable containing one item, which is a null reference. Is this my only option?

I know I could just use a regular function and return the collection once fully iterated. This is an exercise to practice with Iterators.

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
Ama
  • 1,373
  • 10
  • 24
  • 2
    It's normally not a good idea to return an null reference when returning an iterator, an empty one is often fine, or even better if there is an error, let the exception propagate and let the caller catch it. – Alejandro Jan 30 '20 at 21:40
  • I would generally not prevent exceptions to propagate, because I believe they should not be part of flow control, but under specific circumstance there isn't really an easy alternative: for example with the ManagementObjectSearcher, in particular for HDD where property names are chaotic. So are you suggesting I just remove the `Return Nothing` line, which would result in an empty collection if an exception is thrown before iteration begins? I like the idea! – Ama Jan 30 '20 at 21:48
  • 2
    I would agree with that. When working with collections, ideally return an empty collection as opposed to null (Nothing). – Anu6is Jan 30 '20 at 21:50
  • I've never done so; always returned a null reference. Could you develop or redirect me to some further reading on why it is preferable? an accepted answer could also do ;) – Ama Jan 30 '20 at 21:52
  • Returning null (`nothing`) is quite common. Why would you return a collection with one element (or empty) if the query returned no results? You always check for null when you perform a query. See LINQ's methods. – Jimi Jan 30 '20 at 21:59
  • 1
    LINQ actually returns and empty collection in instances where an Enumerable is expected (example `Where`)..empty collection meaning count is 0. For objects, null is returned. It is a somewhat subjective topic however. https://stackoverflow.com/questions/1969993/is-it-better-to-return-null-or-empty-collection – Anu6is Jan 30 '20 at 22:07
  • I would agree with @Anu6is to return an empty collection. Checking for null references in chains of LINQ functions is inconvenient. – djv Jan 30 '20 at 22:09
  • 1
    Yes, but this doesn't look like a collection that needs to support a *chaining* feature. It look more like something you want to return results. So you query and and check for null. Then, if possible, you iterate its content. The destination/usage matters. What's important is that you document its behavior. – Jimi Jan 30 '20 at 22:16
  • 1
    `ProcessorIds.Where(Function(p) True)` throws an exception when the iterator is null. I suppose one could check for null in line with not too much trouble `ProcessorIds?.Where(Function(p) True)`. But yes, the behavior should be documented :) – djv Jan 30 '20 at 22:24

2 Answers2

2

I don't see a way to cause an Iterator to return an empty collection upon exception, if that is what you really want to do. If you do, you could stop using Iterator and Yield, and simply do

Public ReadOnly Property ProcessorIds As IEnumerable(Of String) Implements IHardwareIds.ProcessorIds
    Get
        Try
            Using s As New ManagementObjectSearcher(New SelectQuery("Win32_Processor"))
                Return s.Get().OfType(Of ManagementObject).Select(Function(o) o("ProcessorId").ToString())
            End Using
        Catch
            Return Enumerable.Empty(Of String)
        End Try
    End Get
End Property

Using LINQ without the Iterator actually allows you to remove the For Each and simplify the code.

(If you would rather return null, it's almost the same)

Public ReadOnly Property ProcessorIds As IEnumerable(Of String) Implements IHardwareIds.ProcessorIds
    Get
        Try
            Using s As New ManagementObjectSearcher(New SelectQuery("Win32_Processor"))
                Return s.Get().OfType(Of ManagementObject).Select(Function(o) o("ProcessorId").ToString())
            End Using
        Catch
            Return Nothing
        End Try
    End Get
End Property

however, returning null has a different feel that other Enumerable functions (such as Where, which returns an empty collection when the predicate is not satisfied by any element, as pointed out by @Anu6is).

You can also stop iteration upon the first exception. If Yield is never hit, your result is an empty collection. I think this is more in the spirit of Iterator.

Public ReadOnly Iterator Property ProcessorIds As IEnumerable(Of String) Implements IHardwareIds.ProcessorIds
    Get
        Try
            Using s As New ManagementObjectSearcher(New SelectQuery("Win32_Processor"))
                For Each mo In s.Get()
                    Yield mo("ProcessorId").ToString()
                Next
            End Using
        Catch
            Return ' or Exit Property
        End Try
    End Get
End Property

The C# analog is yield break; and it seems quite clean. See also this answer.

djv
  • 15,168
  • 7
  • 48
  • 72
  • Glad you declared that MOS with a `Using` statement. As a note, the query result objects should be inspected in a `foreach` loop (as shown in the question), then `Dim ProcessorId As String = MyInfo("ProcessorId")?.ToString()` and `yield ProcessorId` if not null. In the end, you'll get an empty collection if no results are found. The empty catch block (in the question) is ugly and not needed. If you catch an exception (you shouldn't; if you do get it, something went wrong at this point), you have to evaluate whether to return null, since you have a partial result caused by an exception. – Jimi Jan 30 '20 at 23:27
1

The best way to handle an issue during setup (rather than during iteration) is to have a public non-iterator wrapper function that deals with the setup and then a private iterator function that does the actual iteration.

I've commonly done this for library-ish functions where I want to validate the arguments, and if you read Jon Skeet's EduLinq series, you'll see that he uses this pattern to handle argument validation.

A simplified example is something like this:

Public Function GetFooStream(ByVal someArg As Bar) As IEnumerable(Of Foo)
    If someArg Is Nothing Then Throw New ArgumentNullException(NameOf(someArg))

    Return _GetFooStreamImpl(someArg)
End Function

Private Iterator Function _GetFooStreamImpl(ByVal someArg As Bar) As IEnumerable(Of Foo)
    'someArg guaranteed to be non-null here
End Function

I just show argument validation here, but this could also include necessary up-front setup as well.

(This is intended as general advice for the case where an Iterator is still the best solution; for this specific case, I'd agree with the preference to use Linq instead of writing your own iteration/transformation.)

Craig
  • 2,248
  • 1
  • 19
  • 23