Skip to main content
replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link
  1. Some of my comments from an answer to a previous questionan answer to a previous question can be carried over directly.

  2. I note that since that previous question you've removed childCount. That's a bit of a shame here, because "Is the size the same?" is a good quick-reject question for equality of collections.

  3. I'm not actually sure whether the implementation of equals is correct or not, because you haven't documented what you consider equality to mean.

    If the intention is that two trees are equal only if they are structurally identical, then you should document that very clearly because it violates the general contract of collections (and although you're not implementing Collection<Integer>, as a user of your class I would assume that your intention was to provide a similar API).

    If your intention is that two trees are equal if they contain the same elements then I strongly suspect that the code is buggy. It seems quite implausible that every possible ordering of insertions would result in an identical structure.

  1. Some of my comments from an answer to a previous question can be carried over directly.

  2. I note that since that previous question you've removed childCount. That's a bit of a shame here, because "Is the size the same?" is a good quick-reject question for equality of collections.

  3. I'm not actually sure whether the implementation of equals is correct or not, because you haven't documented what you consider equality to mean.

    If the intention is that two trees are equal only if they are structurally identical, then you should document that very clearly because it violates the general contract of collections (and although you're not implementing Collection<Integer>, as a user of your class I would assume that your intention was to provide a similar API).

    If your intention is that two trees are equal if they contain the same elements then I strongly suspect that the code is buggy. It seems quite implausible that every possible ordering of insertions would result in an identical structure.

  1. Some of my comments from an answer to a previous question can be carried over directly.

  2. I note that since that previous question you've removed childCount. That's a bit of a shame here, because "Is the size the same?" is a good quick-reject question for equality of collections.

  3. I'm not actually sure whether the implementation of equals is correct or not, because you haven't documented what you consider equality to mean.

    If the intention is that two trees are equal only if they are structurally identical, then you should document that very clearly because it violates the general contract of collections (and although you're not implementing Collection<Integer>, as a user of your class I would assume that your intention was to provide a similar API).

    If your intention is that two trees are equal if they contain the same elements then I strongly suspect that the code is buggy. It seems quite implausible that every possible ordering of insertions would result in an identical structure.

Source Link
Peter Taylor
  • 24.5k
  • 1
  • 49
  • 94

  1. Some of my comments from an answer to a previous question can be carried over directly.

  2. I note that since that previous question you've removed childCount. That's a bit of a shame here, because "Is the size the same?" is a good quick-reject question for equality of collections.

  3. I'm not actually sure whether the implementation of equals is correct or not, because you haven't documented what you consider equality to mean.

    If the intention is that two trees are equal only if they are structurally identical, then you should document that very clearly because it violates the general contract of collections (and although you're not implementing Collection<Integer>, as a user of your class I would assume that your intention was to provide a similar API).

    If your intention is that two trees are equal if they contain the same elements then I strongly suspect that the code is buggy. It seems quite implausible that every possible ordering of insertions would result in an identical structure.