Cleaning up 3D bin packing [closed]

Question

This is a follow up question to 3D bin packing in Java

I finished testing my code's result and started cleaning it up.

Could anyone please tell me what I should do or fix to clean up this code more?

I'm required to put a comment for every instruction line I write. English is not my first language, so maybe some of the comments are not clear enough.

private int ID;
private double length;
private double height;
private double breadth;
private ArrayList<ItemsUnit> packedItems;
private double x, y, z = 0;
private double[] remainingLength;
private double[] remainingHeight;
private double[] remainingBreadth;


/**
 * initialize the remaining dimensions' arrays
 * @param n
 */
public void init(final int n) {

    remainingLength = new double[n];
    remainingHeight = new double[n];
    remainingBreadth = new double[n];

    for (int i = 0; i < n; i++) {

        /*
         * initialize the remainingLength array to the length of the container
         */
        remainingLength[i] = length;

        /*
         * initialize the remainingHeight array to the height of the container
         */
        remainingHeight[i] = height;

        /*
         * initialize the remainingBreadth array to the breadth of the container
         */
        remainingBreadth[i] = breadth;
    }
}


/**
 * put3D method
 * @param item
 * @param p
 * @param n
 * @return
 */
public boolean put3D(final ItemsUnit item, final int p, final int n) {

    /*
     * if x still did not exceed the container's length
     */
    if (x < length) {

        z = remainingHeight[p];     // update z to the position of the new item added
        y = remainingBreadth[p];    // update y to the position of the new item added

        /*
         * if we can put the new item next to the item packed in the extreme point of length
         */
        if (putL(item, p)) {

            packedItems.add(item);  // if item fits add it to the packedItems into the container
            return true;
        }
    }

    /*
     * if x still did not exceed the container's breadth
     */
    if (y < breadth) {

        x = remainingLength[p];     // update x to the position of the new item added
        z = remainingHeight[p];     // update z to the position of the new item added

        /*
         * if we can put the new item next to the item packed in the extreme point of breadth
         */
        if (putB(item, p)) {

            packedItems.add(item);  // if item fits add it to the packedItems into the container
            return true;
        }
    }

    /*
     * if z still did not exceed the container's height
     */
    if (z < height) {

        x = remainingLength[p];     // update x to the position of the new item added
        y = remainingBreadth[p];    // update y to the position of the new item added

        /*
         * if we can put the new item next to the item packed in the extreme point of height
         */
        if (putH(item, p)) {

            packedItems.add(item);  // if item fits add it to the packedItems into the container
            return true;
        }
    }

    return false;   //return false if item cannot be packed in neither extreme point
}


/**
 * adding the new item to the extreme point in length
 * @param item
 * @param p
 * @return
 */
private boolean putL(final ItemsUnit item, final int p) {

    double minRemL = remainingLength[0];     //the minimum remaining length of all already packed items
    int i = 0;  //to store the index of the item next to which we should put the new item

    /*
     * choosing the point (position) where to put the new item
     */
    for (int j = 0; j < remainingLength.length; j++) {

        if ((remainingLength[j] != 0) && (minRemL >= remainingLength[j])
                && (remainingLength[j] >= item.getLength())) {

            i = j;  //storing the item next to which we should put the new packed item
            minRemL = remainingLength[j];   //minimum length left

        }
    }

    remainingLength[p] = remainingLength[i] - item.getLength();   //update the remaining length of the new item added 
    remainingBreadth[p] -= item.getBreadth();   //update the remaining breadth of the new item added
    remainingHeight[p] -= item.getHeight();     //update the remaining height of the new item added
    remainingLength[i] = 0;      //insert 0 to the remainingLength of the item next to which we put the new item (so that we don't consider its remaining length anymore)

    x += item.getLength();  //increment x by the length of the new packed item in the extreme point of length

    return true;
}

/**
 * adding the new item to the extreme point in breadth
 * @param item
 * @param p
 * @return
 */
private boolean putB(final ItemsUnit item, final int p) {

    double minRemB = remainingBreadth[0];    //the minimum remaining breadth of all already packed items
    int i = 0;  //to store the index of the item next to which we should put the new item

    /*
     * choosing the point (position) where to put the new item----
     */
    for (int j = 0; j < remainingBreadth.length; j++) {

        if ((remainingBreadth[j] != 0) && (minRemB >= remainingBreadth[j])
                && (remainingBreadth[j] >= item.getBreadth())) {

            i = j;  //choosing the item to which we should put the new packed item next to
            minRemB = remainingBreadth[j];   //minimum length left

        }
    }

    remainingBreadth[p] = remainingBreadth[i] - item.getBreadth();   //update the remaining breadth of the new item added 
    remainingHeight[p] -= item.getHeight(); //update the remaining height of the new item added
    remainingLength[p] -= item.getLength();     //update the remaining length of the new item added
    remainingBreadth[i] = 0;    //insert 0 to the remainingBreadth of the item next to which we put the new item (so that we don't consider its remaining breadth anymore)

    y += item.getBreadth();     //increment y by the breadth of the new packed item in the extreme point of breadth

    return true;
}


/**
 * adding the new item to the extreme point in height
 * @param item
 * @param p
 * @return
 */
private boolean putH(final ItemsUnit item, final int p) {

    double minRemH = remainingHeight[0];    //the minimum remaining height of all already packed items
    int i = 0;  //to store the index of the item next to which we should put the new item

    /*
     * choosing the point (position) where to put the new item----
     */
    for (int j = 0; j < remainingHeight.length; j++) {

        if ((remainingHeight[j] != 0) && (minRemH >= remainingHeight[j])
                && (remainingHeight[j] >= item.getHeight())) {

            i = j;   //choosing the item to which we should put the new packed item next to
            minRemH = remainingHeight[j]; //minimum length left

        }
    }

    remainingHeight[p] = remainingHeight[i] - item.getHeight();  //update the remaining height of the new item added 
    remainingBreadth[p] -= item.getBreadth();   //update the remaining breadth of the new item added
    remainingLength[p] -= item.getLength();     //update the remaining length of the new item added
    remainingHeight[i] = 0;     //insert 0 to the remainingHeight of the item next to which we put the new item (so that we don't consider its remaining height anymore)

    z += item.getHeight();  //increment z by the height of the new packed item in the extreme point of height

    return true;
}

Answer 1

Broken Code

Your code is broken right now.

If I make a 2 by 2 by 2 cube.

And I put in it a 3 by 1 by 1 stick.

Then you say "sure, okay!"

/*
 * if x still did not exceed the container's length
 */
if (x < length) {

    z = remainingHeight[p];     // update z to the position of the new item added
    y = remainingBreadth[p];    // update y to the position of the new item added

    /*
     * if we can put the new item next to the item packed in the extreme point of length
     */
    if (putL(item, p)) {

        packedItems.add(item);  // if item fits add it to the packedItems into the container
        return true;
    }

x < length, well, length is 2, x is 0. So that's true.

z and y are set to 0, because that's the spot you're putting the item in.

Then we call putL, which ALWAYS returns true.

So even if the item doesn't fit, you can always put it in.

Taking a closer look at your previous questions, I think you should go back to the code you had in the first question you asked, and make sure that sticking a 3x1x1 stick in a 2x2x2 cube gives you "false".

---

So when your boss or your teacher asks you to comment every line, there's not much to do about it.

That doesn't take away that comments are usually used to explain "why" something was done in the code. Your teacher asks you to add a comment for each line, so explain "why & how", to show both your thought process and your solution.

Specifically...

/**
 * initialize the remaining dimensions' arrays
 * @param n
 */
public void init(final int n) {

What does n do?

/**
 * put3D method
 * @param item
 * @param p
 * @param n
 * @return
 */
public boolean put3D(final ItemsUnit item, final int p, final int n) {

Or item? p? n again? Is that n the same n or a different one? What does the boolean I get back mean?

Local variable declarations too:

int i = 0;  //to store the index of the item next to which we should put the new item

Don't use i here; write a descriptive variable name!

init in general is flawed. What you should use is a constructor, taking a width, height and length of the container. Then you can set the arrays and such based on those values. Right now, init only allows creating cubes that are the same size in all dimensions... if I'm reading your code correctly.