X

Aggregator

CodeSOD: Connection State

The Daily WTF
2 months 4 weeks ago

Frederick A sends us a bit of null checking code, and offers us a better solution.

class ConferenceService { /// <summary> /// Checks if conference is active /// </summary> public bool IsCalling() { try { return m_ConnectionService.Core.State.IsWebRTCConnected; } catch { return false; } } }

This is for a web conferencing tool, which uses WebRTC to set up connections between clients in the chat. This function checks if the chat is active by checking a IsWebRTCConnected flag. But as you can see in this code, that flag is on a long chain of objects, some of which may not exist when this function is called. Thus, we wrap the whole thing up in a try/catch. If anything throws an exception, we know we can just return false. It's probably fine.

The obvious and easy fix, which Frederick proposes, is to use the C# coalescing operator: ?. m_ConnectionService?.Core?.State?.IsWebRTCConnected ?? false would solve this problem just fine.

That said, I wouldn't say that's a true fix. We're talking about a state machine here, though admittedly with two states under discussion (connected/disconnected), though there are probably more not being checked here. This information should be managed via a state machine, not via boolean flags stuffed deep in an object chain. The fix isn't a WTF, but it definitely hints at a better way to manage all of this. Now, my solution likely requires a lot more modification and code changes than what we have here, so I'm not suggesting anyone go off and rewrite this from scratch just to have a cleaner way of managing state. But folks definitely should think more carefully about how they manage state.

.comment { border: none; } [Advertisement] Keep all your packages and Docker containers in one place, scan for vulnerabilities, and control who can access different feeds. ProGet installs in minutes and has a powerful free version with a lot of great features that you can upgrade when ready.Learn more.
Remy Porter