Amazon Black Hat coding question

I’m at Black Hat, hanging out at the Amazon booth with my colleagues, and quite amazed at the stir created by the coding question I put up on the board.

Vegas 017

For those of you that haven’t had a chance to see the code, or need more time to figure out what’s wrong, I’m posting the code here. Sadly, no prizes for those of you who aren’t able to come to the booth, and I won’t be accepting posts commenting on the code until after the conference is over.Vegas 015

Consider it a theoretical exercise in code review – how would you handle this Java code arriving in front of you at a code review?

public static boolean isDifferent(
    final MyBean oldBean,
final MyBean newBean)
        oldBean != newBean &&
oldBean != null &&

Post your comments, and after the conference is over, I’ll open them up.

For those of you that think in C++ instead of Java (and there were many at the conference), here’s a version in that language:

    static bool isDifferent(
        const someClass const * oldObject, 
        const someClass const * newObject)
            oldObject != newObject &&
            oldObject != NULL &&

There are no ‘cheat’ issues – code not present in the sample is assumed to be perfect.

8 thoughts on “Amazon Black Hat coding question”

  1. I see only semantic problems.

    1. old->equals probably means old->isEqual, if not it might try to make newObject equal to old or the other way round. Which btw. fails because of the ‘final’

    No, .equals is defined at (and other places) as a comparison operator, designed not to change either value.

    2. isDifferent(obj, obj) with the same object on both parameters will give False, which is as well very counter-intuitive.

    That’s certainly a bad thing, and would be worth bringing up in code review – perhaps even to the extent of rejecting the function’s name and use. But it’s not the actual flaw.
    Tuesday, August 31, 2010 16:10 PM by Alun Jones
  2. It returns false if oldBean and newBean are the same object.

    It should eliminate that first expression from the return statement (oldBean != newBean). equals() is sufficient.

    OK, no.

    isDifferent(a,a) should return false, so your first sentence is incorrect. Your second sentence doesn’t fix the function either, and may actually introduce more errors.

    Tuesday August 31, 2010 16:14 PM by Alun Jones
  3. Fails for
    isDiff ( null, obj )
    expected: true
    actual: false

    How abt the following?
    oldBean != newBean &&
     ( oldBean == null
               newBean == null
            ! oldBean.equals(newBean) ); 

  4. Is there an official correct answer?

    re: Official answers…


    I’m not quite ready to post anything that I would call “official answers” yet – but you should be able to see some answers above that you can demonstrate to yourself as correct.

    Perhaps the best answer, however, is that code like this should have been rejected without even evaluating if it is correct in function, because it is so hard for average developers to read.


    Tuesday, December 14, 2010 6:55 AM by Alun Jones

Leave a Reply

Your email address will not be published. Required fields are marked *