Skip to main content

CodeSOD: On Deep Background

3 months 1 week ago

Andrew worked with Stuart. Stuart was one of those developers who didn't talk to anyone except to complain about how stupid management was, or how stupid the other developers were. Stuart was also the kind of person who would suddenly go on a tear, write three thousand lines of code in an evening, and then submit an pull request. He wouldn't respond to PR comments, however, and just wait until management needed the feature merged badly enough that someone said, "just approve it so we can move on."

.comment {border: none;} int iDisplayFlags = objectProps.DisplayInfo.BackgroundPrintFlags; bool bForceBackgroundOn = false; bool bForceBackgroundOff = false; // Can't use _displayTypeID because it will always be 21 since text displays as image if (_fileTypeID == 11) // TEXT { if ((iDisplayFlags & 0x1008) != 0) // Text Background is required { bForceBackgroundOn = true; } else if ((iDisplayFlags & 0x1001) != 0) // Text Background is not available { bForceBackgroundOff = true; } } else if (_displayTypeID == 21) // IMAGE { if ((iDisplayFlags & 0x1200) != 0) // Image Background is required { bForceBackgroundOn = true; } else if ((iDisplayFlags & 0x1040) != 0) // Image Background is not available { bForceBackgroundOff = true; } } bool useBackground = bForceBackgroundOn; // If an object does not have an Background and we try to use it, bad things happen. // So we check to see if we really have an Background, if not we don't want to try and use it if (!useBackground && objectProps.DisplayInfo.Background) { useBackground = Convert.ToBoolean(BackgroundShown); } if (bForceBackgroundOff) { useBackground = false; }

This code is inside of a document viewer application. As you might gather from skimming it, the viewer will display text (as an image) or images (as an image) and may or may not display a background as part of it.

This code, of course, uses a bunch of magic numbers and bitwise operators, which is always fun. We don't need any constants. It's important to note that all the other developers on the project did use enumerations and constants. The values were defined and well organized in the code- Stuart simply chose not to use them.

You'll note that there's some comments and confusion about how we can't use _displayTypeID because text always displays as an image. I'm going to let Andrew explain this:

The client this code exists in renders text documents to images (for reasons that aren’t relevant) when presenting them to the user. We have a multitude of filetypes that we do similar actions with, and fileTypes are user configurable. Because of this, we also keep track of the display type. This allows the user to configure a multitude of filetypes, and depending on the display type configured for the file type, we know if we can show it in our viewer. In the case of display type ‘text’ our viewer ultimately renders the text as an image. At some point in time Stuart decided that since the final product of a text document is an image, we should convert display type text over to image when referencing it in code (hence the comment ‘Can’t use display type ID’). If none of this paragraph makes any sense to you, then you’re not alone, because the second someone competent got wind of this, they thankfully nixed the idea and display type text, went back to meaning display type text (aka this goes through OUR TEXT RENDERER).

What I get from that paragraph is that none of this makes sense, but it's all Stuart's fault.

What makes this special is that the developer is writing code to control a binary status: "do we show a background or not?", but needs two booleans to handle this case. We have a bForceBackgroundOn and a bForceBackgroundOff.

So, tracing through, if we're text and any of the bits 0x1008 are set in iDisplayFlags, we want the background on. Otherwise, if any of the bits 0x1001 are set, we want to force the background off. If it's an image, we do the same thing, though for 0x1200 and 0x1040 respectively.

Then, we stuff bForceBackgroundOn into a different variable, useBackground. If that is false and a different property flag is set, we'll check the value of BackgroundShown- which we choose to convert to boolean which implies that it isn't a boolean, which raises its own questions, except it actually is a boolean value, and Stuart just didn't understand how to deal with a nullable boolean. Finally, after all this work, we check the bForceBackgroundOff value, and if that's true, we set useBackground to false.

I'll be frank, none of this quite makes sense to me, and I can certainly imagine a world where the convoluted process of having a "on" and "forceOff" variable actually makes sense, so I'd almost think this code isn't that bad- except for this little detail, from Andrew:

The final coup de grace is that all of the twisted logic for determining if the background is needed is completely unnecessary. When the call to retrieve the file to display is made, another method checks to see if the background was requested (useBackground), and performs the same logic check (albeit in a sane manner) as above.

The code is confusing and unnecessary.

[Advertisement] Picking up NuGet is easy. Getting good at it takes time. Download our guide to learn the best practice of NuGet for the Enterprise.
Remy Porter

Free 'T-Mobile Starlink' for Six Months Announced During Super Bowl. Also Available to Verizon and AT&T Customers

3 months 1 week ago
Today T-Mobile announced what they're calling "the next big thing in wireless" — T-Mobile Starlink. But the real surprise is "The beta is now open for absolutely everyone — yes, even Verizon and AT&T customers — to register for free access until July." And, as they explained to Americans watching the Super Bowl, "If you can see the sky you're connected." Now in public beta, this breakthrough service, developed in partnership with Starlink, uses straight-out-of-a-sci-fi-movie satellite and mobile communications technology to help keep people connected — even you, Verizon and AT&T customers — in the more than 500,000 square miles of the country unreached by any carrier's earth-bound cell towers. That's nearly the size of two Texases...! The beauty of the service is its simplicity: users don't need to do anything out of the ordinary. When a user's cell phone gets out of range of a cell tower, the phone automatically connects to the T-Mobile Starlink network. No need to manually connect. Messages are sent and received just as they are today on a traditional network, even group texts and reactions. And it works on most smartphones from the last four years. It's not limited to a few smartphones or operating systems... The beta is free until July at which point T-Mobile Starlink will be included at no extra cost on Go5G Next (including variations like Go5G Next 55+), T-Mobile's best plan. Business customers will also get T-Mobile Starlink at no extra cost on Go5G Business Next, first responder agencies on T-Priority plans and other select premium rate plans. T-Mobile customers on any other plan can add the service for $15/month per line. Through February, T-Mobile customers who have registered for the beta can secure a $10/month per line Early Adopter Discount, 33% off the full price. AT&T and Verizon customers hate dead zones, too When your service is amazing and different, you want as many people to try it as possible. T-Mobile is giving AT&T and Verizon customers the opportunity to try out T-Mobile Starlink satellite service on their existing phones... During the beta period, Verizon and AT&T customers can experience T-Mobile Starlink text messaging for free, and once the service launches in July, it will be available for $20/month per line... More details and consumer registration can be found here. A Vision for Universal Coverage As T-Mobile and Starlink continue to work towards eliminating mobile deadzones, the companies welcome wireless providers from around the world to join their growing alliance, which aims to provide reciprocal roaming for all participating carriers. So far, KDDI (Japan), Telstra (Australia), Optus (Australia), One NZ (New Zealand), Salt (Switzerland), Entel (Chile & Peru), Rogers (Canada) and Kyivstar (Ukraine) are among the providers that have signed on to join the cause and launch satellite-to-mobile technology. Learn more about the alliance and how providers can join at direct.starlink.com.

Read more of this story at Slashdot.

EditorDavid