14
Awlex
7y

My eyes...

Comments
  • 2
    "I don't want to delete it. Maybe I need this later!".
  • 0
    @Makenshi It's more about these if brackets.

    Simple and short solution:

    If (nowItem = parser.getLocalName().equals("item"))
    item = new Item();
  • 0
    I should be a code reviewer
  • 2
    @Awlex I once too assigned variables in if statements, until I had this one bug that I could not figure out for hours that was caused by my eyes mistaking the assignment for a comparison. Don't. Just don't assign variables in if statements.
  • 0
    Love that long switch
  • 3
    @Awlex Oh ok. I thought you meant the "zombie code". Although I disagree with your statement. It's a bad practice to assign values in if blocks. Also the comparisons should be "string".equal(value) to avoid NPE.

    For code reviews you should be aware of those things. ;-) Also the long switch construction is really ugly.
  • 0
    @Makenshi I know, that this long switch isn't an eyecatcher, but Is there really a better way when parsing XML manually?
  • 2
    @Awlex In case of XML parsing I would go with the approach to just extract the code from the case statements into methods with descriptive names. Normally this would be to brittle, as you have to would have to modify the switch statement each time a new "case" is needed. Since the number of XML tokens in your SAX parser is known, this would seem ok. You could maybe take a look at the Handler api from SAX2. It offers separate methods for the SAX parsing approach.
Add Comment