CodeSOD: Break Out of your Parents

When I first glanced at this submission from Thomas, I almost just scrolled right by. "Oh, it's just another case where they put the same code in both branches of the conditional," I said. Then I looked again.

if (obj.success) { //#BZ7350 for (i = 0; i < parent.length; i++) { try { parent[i]['compositionExportResultMessage'](obj.success, obj.response, 'info'); break; } catch (e) { } }}else { //#BZ7350 for (i = 0; i < parent.length; i++) { try { parent[i]['compositionExportResultMessage'](obj.success, obj.response, 'error'); break; } catch (e) { } }}

First, I want to give a little shout out to my "favorite" kind of ticket management: attaching a ticket number to a comment in your code. I can't be certain, but I assume that's what //#BZ7350 is doing in there, anyway. I've had the misfortune of working in places that required that, and explaining, "I can just put it in my source control comments," couldn't shift company policy.

Now, this is a case where nearly identical code is executed in either branch. The key difference is whether you pass info or error up to your parent.

Which" parent is apparently an array, which means it should at least be called parents, but either way that ends up being a weird choice. Sure, there are data structures where children can have multiple parents, but how often do you see them? Especially in web programming, which this appears to be, which mostly uses trees.

But fine, a child might have multiple parents. Those parents may or may not implement a compositionExportResultMessage, and if they do, it may or may not throw an exception when called. We want hopefully one parent to handle it, so take a close look at the loop.

We try to call compositionExportResultMessage on our parent. If it's successful, we break. If it fails, we throw the exception away and try with the next parent in our array. Repeat until every parent has failed, or one has succeeded.

Thomas didn't provide much context here, but I'm not sure how much we actually need. Something is really wrong in this code base, and based on how ticket #BZ7350 was closed, I don't think it's gonna get any better.

[Advertisement] ProGet can centralize your organization's software applications and components to provide uniform access to developers and servers. Check it out!