• LZ79LRU (unregistered)

    Assuming that the logic of the code is correct, which it probably is the only sin here is not wanting to use nested try / catch blocks with catch actually doing what it's supposed to. But to be fair there, we are all so used to catch being used only to log and rethrow that it is understandable if unfortunate that someone might feel anxiety about writing "productive" code in a catch block.

  • (nodebb)

    One possibility is that the Framework function sometimes returns null and sometimes throws an exception. Which would make the framework TRWTF.

  • (nodebb)

    The only reason I can think of for the two calls to get the image is the framework might be doing something that the first SHOULD work, but maybe too small of a timeout value makes the first call fail like 90% of the time, and the second call happens to use the cached data created by the first call, which, if allowed to run 10ms more, would allow the image to be received.

  • (nodebb)

    They first try to fetch a localized image, if that fails they try to get a default one and if that fails they just return a white Brush. The way I see it the exception isn't captured because it isn't relevant and the code is implemented with the implication that some or all requested images will be missing.

    The part that actually caught my attention was the "BorderChannelGroupBrush" variable which looks like it was an instance o maybe a static made local, probably as a result of a change in requirements.

  • Hal (unregistered)
    Comment held for moderation.
  • PedanticRobot (unregistered)
    Comment held for moderation.
  • TheCPUWizard (unregistered)
    Comment held for moderation.
  • (nodebb) in reply to LZ79LRU

    I was thinking the same. If "boomerang code" is bad, "boomerang code in catch blocks" is worse.

  • (nodebb)

    At least it only took until the fourth post for someone to remark that it isn't two attempts to fetch the same image, but one for the localised image (frobble-en.jpg) and one for the non-localised image (frobble.jpg).

    The thing that makes my teeth itch, however, is that there is no way to see at runtime (when compiled in debug mode, naturally) that we failed to fetch both images.

    Even if it's just the equivalent of an ifdef'ed call to OutputDebugString in the catch blocks, it would be nice to have some indication that it failed, and even better, why.

    Addendum 2025-01-15 12:11: Note: the indication is to distinguish "we downloaded a white image" from "we failed totally to download an image"

  • (nodebb)

    I think this one is actually reflecting for the real WTF. First off, there's nothing really wrong with getting the index of the extension, especially since this doesn't seem to be some random images but some assets that get loaded into this obvious ancient WinForm app from a remote server.

    However ServicePageImageUrlOnContentServer is the weirdest method name I have ever read. It's just a collection of nouns. So this puts a lot into perspective and I think the main issue might even be found there. I guess this methods just doesn't do proper exception handling, fails with improper exceptions, falls through with null, is not deterministic, all the fun stuff.

    So yeah, this code will not get any beauty points for sure but heck would it be interesting to get a peek into this weird method.

    Addendum 2025-01-15 12:14: WPF app, not WinForm obviously.

  • (nodebb) in reply to LZ79LRU

    it is understandable if unfortunate that someone might feel anxiety about writing "productive" code in a catch block

    I try to avoid doing that because an uncaught exception in a catch block terminates the program immediately.

Leave a comment on “Brushing Up”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article