" '—All You Zombies—' "
-
boolean parse(String xml) { Result result = new Result(); XmlParser parser = new XmlParser(result); parser.processDocument(xml); ResultXML resultXml = result.getResult(); parser = new XmlParser(this.result); parser.processDocument(resultXML.getXml()); return true; }
Error handling redacted for brevity.
-
Are we playing "guess the WTFs"?
Sure, having to parse an XML twice looks idiotic, but might there even be something else going on?
For example, where does this theXmlParser
come from, is this a home-rolled parser or some library?
-
I think it's the fact that it does all that and just returns a boolean in the end. Instead of, you know, a parsed document.
-
It's stored in
this.result
, no?
-
Seems like it does. That's... ummm... one way to do it.
-
Oh, I missed that.
Well then... ummm... without knowing more about the language/framework here I'm stumped as well.
-
It's hard to see without context but even if it does put the result into
this.result
- it seems like it might as well just return it rather than the bool.
-
XmlParser is in our code. I haven't looked, but I do expect it to contain a real XML parser.
The weirdest thing to me was creating a result object, initializing the parser with it, then getting the ‘result’ property of that result. Hasn't anybody here ever heard of return values?
-
Hasn't anybody here ever heard of return values?
Maybe they all have to be boolean?
On a more serious note, maybe they are relying on checking booleans rather than checking for empty or null objects, assuming that's what you'd return on parsing failure?
-
Notice there's only one return statement. The only other way out of this method is through the redacted error handling code (as in: runtime exceptions).Shit, no, I'm wrong.
catch (Exception e) { return false; }
-
Ok, that is stupid. I assumed there would be more returns in the redacted bit because, you know, sanity.
-
-
Where's @Gaska to wax poetic about the use of error checking here instead of exceptions?
-
Aha! See? SANIT
Nope, could not finish that without bursting into tears.
-
Hmm, my guess is this is supposed to parse the document, save the result to
this.result
and return true. Except, he forgot to do the last step.And then there's the last 3 lines, where he's parsing it again. I guess some kind of error checking?
-
and return true
he forgot to do the last step.
return true;
Que?
-
@Buddy said:
catch (Exception e) { return false; }
Because Belgium the Support team, that'll show 'm.
-
Yeah, it's an XML document that contains an entire other XML document as a string. I already knew that when I came to the method. I suppose I should have mentioned that.
All I wanted to know was where the file was coming from and where the result was going. I thought I would have been in and out of a method like this in under 5 seconds—or better yet, not have to read the
parse
method at all—but instead it's all results being assigned to random properties and throwing exceptions just to catch them again 5 lines later and I can't follow the flow of this stuff for shit.
-
Imma ignore the flame war and mention that I now have this song stuck in my head...
www.youtube.com/watch?v=2LE0KpcP05I
And I still can't make youtube videos onebox...
-
You need to include http://
http://www.youtube.com/watch?v=2LE0KpcP05I
-
Imma ignore the flame war and mention that I now have this song stuck in my head...
Oh good, it's not just me. It's been driving me crazy.
-
I handled 33 errors to an existing exception: C vs C++, from the author of ZeroMQ
-
Blame @Yamikuronue.
How about I blame you, and IconProUK, the two idiots who actually did the fucking thing? Get the fuck out of here with your stupid argument.
-
How about I blame you, and IconProUK, the two idiots who actually did the fucking thing?
Agreed.
Get the fuck out of here with your stupid argument.
It's been moved now.
-
-
Out of curiosity, why Icon?
Out of curiosity, why 436 posts about logging?
Don't ask questions you:
-
Already know the answer to
-
Don't want to know the answer to
That question is one or both, depending on how much you've been paying attention around here. (Meaning: almost certainly 2. Due to goldfish-memory.)
-
-
The only other use I can find is here, and there's no explanation there, so…
-
How about I blame you, and IconProUK, the two idiots who actually did the fucking thing?
Blame whoever you want. As if I care.Get the fuck out of here with your stupid argument.
OK, but let me replace it with another one. With you.Out of curiosity, why 436 posts about logging?
First, not 436 but 293. Second, it wasn't about logging but error handling.Don't ask questions you:
-
Already know the answer to
-
Don't want to know the answer to
Are your shoulder aliens capable of reading minds now? Because how else would you know what he knows or wants?
-
-
Damn that means I've used the same one twice. Oh well.
-
It's stored in
this.result
, no?No.
Result result = new Result()
. That's not a member. It's local to the function.EDIT: Oh. Duh...
-
(code below from original post)
boolean parse(String xml) { Result result = new Result(); XmlParser parser = new XmlParser(result); parser.processDocument(xml); ResultXML resultXml = result.getResult(); parser = new XmlParser(this.result); // see below parser.processDocument(resultXML.getXml()); return true; }
I realize this is kind of late, but ...
Maybe I missed something, but I think there's an error in this. The this.result in the line I marked with a comment: Shouldn't that be
this.resultXml?After note: It shouldn't have this. on it; it should just be resultXml, I think. Sorry for the glitch.
-
That's the trick here, is that this.result implements the same interface as the function-local result. So when I skimmed through this trying to find how result gets set I'm like “ok, you put
result
into an XML parser, then you parse it, and you get an XML result. Then you parse the XML result and the result of that goes intoresult
? Wtf?”
-
That's the trick here...
Oh.
That is truly creative, and I had a lot of trouble ferreting that out even on the second pass. There is just nothing like like truly "readable" code, is there?
-
Error handling redacted for brevity.
And confusion. (God, I hate working with XML in Java, especially at the DOM level.)
-
Found the bug.
public boolean doWork() { try { process(); return true; } catch (InteractionException ex) { getLogger().logError("InteractionException caught", ex); doRollbackCleanUp(); } catch (Exception e) { getLogger().logError("Error", e); } return false; } void meanwhileInAnotherFile() { //set up the workflow workflow.doWork(); //continue on without so much as a glance at the return value }
-
:HowNotToDoIt.class: