Skip to main content
added 286 characters in body
Source Link
Pimgd
  • 22.6k
  • 5
  • 68
  • 144

A minor bug: You get an IndexOutOfBoundsException for an empty list in SwapTreeLevelsValues.create(List<T> items). You don't have a comment stating you need to input a list containing at least something. Consider returning IllegalArgumentException and adding a comment.


I had to do a double take here:

final int left = 2 * i + 1;
final int right = 2 * i + 2;

It only made sense after looking at the diagram. Still, you're doing a calculation twice... why not do it like this?

final int left = 2 * i + 1;
final int right = left + 1;

That said, you might wanna put a comment there. It's not a trivial line of code.

Looking up a bit...

final int half = items.size() / 2;

for (int i = 0; i < half; i++) {
    if (items.get(i) != null) {

You're not modifying items. Is it possible for items.get(i) to not return null once it has once returned null? If not, why not break out of the for loop, rather than going through the remaining loops? (Performance when dealing with large TreeMaps).

I can't find anything else, really.

I had to do a double take here:

final int left = 2 * i + 1;
final int right = 2 * i + 2;

It only made sense after looking at the diagram. Still, you're doing a calculation twice... why not do it like this?

final int left = 2 * i + 1;
final int right = left + 1;

That said, you might wanna put a comment there. It's not a trivial line of code.

Looking up a bit...

final int half = items.size() / 2;

for (int i = 0; i < half; i++) {
    if (items.get(i) != null) {

You're not modifying items. Is it possible for items.get(i) to not return null once it has once returned null? If not, why not break out of the for loop, rather than going through the remaining loops? (Performance when dealing with large TreeMaps).

I can't find anything else, really.

A minor bug: You get an IndexOutOfBoundsException for an empty list in SwapTreeLevelsValues.create(List<T> items). You don't have a comment stating you need to input a list containing at least something. Consider returning IllegalArgumentException and adding a comment.


I had to do a double take here:

final int left = 2 * i + 1;
final int right = 2 * i + 2;

It only made sense after looking at the diagram. Still, you're doing a calculation twice... why not do it like this?

final int left = 2 * i + 1;
final int right = left + 1;

That said, you might wanna put a comment there. It's not a trivial line of code.

Looking up a bit...

final int half = items.size() / 2;

for (int i = 0; i < half; i++) {
    if (items.get(i) != null) {

You're not modifying items. Is it possible for items.get(i) to not return null once it has once returned null? If not, why not break out of the for loop, rather than going through the remaining loops? (Performance when dealing with large TreeMaps).

I can't find anything else, really.

added 84 characters in body
Source Link
Pimgd
  • 22.6k
  • 5
  • 68
  • 144

I had to do a double take here:

final int left = 2 * i + 1;
final int right = 2 * i + 2;

It only made sense after looking at the diagram. Still, you're doing a calculation twice... why not do it like this?

final int left = 2 * i + 1;
final int right = left + 1;

That said, you might wanna put a comment there. It's not a trivial line of code.

Looking up a bit...

final int half = items.size() / 2;

for (int i = 0; i < half; i++) {
    if (items.get(i) != null) {

You're not modifying items. Is it possible for items.get(i) to not return null once it has once returned null? If not, why not break out of the for loop, rather than going through the remaining loops? (Performance when dealing with large TreeMaps).

I can't find anything else, really.

I had to do a double take here:

final int left = 2 * i + 1;
final int right = 2 * i + 2;

It only made sense after looking at the diagram. Still, you're doing a calculation twice... why not do it like this?

final int left = 2 * i + 1;
final int right = left + 1;

Looking up a bit...

final int half = items.size() / 2;

for (int i = 0; i < half; i++) {
    if (items.get(i) != null) {

You're not modifying items. Is it possible for items.get(i) to not return null once it has once returned null? If not, why not break out of the for loop, rather than going through the remaining loops? (Performance when dealing with large TreeMaps).

I can't find anything else, really.

I had to do a double take here:

final int left = 2 * i + 1;
final int right = 2 * i + 2;

It only made sense after looking at the diagram. Still, you're doing a calculation twice... why not do it like this?

final int left = 2 * i + 1;
final int right = left + 1;

That said, you might wanna put a comment there. It's not a trivial line of code.

Looking up a bit...

final int half = items.size() / 2;

for (int i = 0; i < half; i++) {
    if (items.get(i) != null) {

You're not modifying items. Is it possible for items.get(i) to not return null once it has once returned null? If not, why not break out of the for loop, rather than going through the remaining loops? (Performance when dealing with large TreeMaps).

I can't find anything else, really.

Source Link
Pimgd
  • 22.6k
  • 5
  • 68
  • 144

I had to do a double take here:

final int left = 2 * i + 1;
final int right = 2 * i + 2;

It only made sense after looking at the diagram. Still, you're doing a calculation twice... why not do it like this?

final int left = 2 * i + 1;
final int right = left + 1;

Looking up a bit...

final int half = items.size() / 2;

for (int i = 0; i < half; i++) {
    if (items.get(i) != null) {

You're not modifying items. Is it possible for items.get(i) to not return null once it has once returned null? If not, why not break out of the for loop, rather than going through the remaining loops? (Performance when dealing with large TreeMaps).

I can't find anything else, really.