0

I'm working in Java right now, and I'm attempting to make an array copy itself into a temporary array, and then overwriting again with a new list, however the code isn't functioning.

MobilePhoneInventory.java

public void delete(MobilePhone p) {
    MobilePhone[] temp = new MobilePhone[list.length-1];
    int adj = 0;
    for (int i = 0; i < list.length; i++) {
        if (p != list[i]) {
            temp[i-adj] = list[i];
        }
        else adj = 1;
    }
    list = temp;
    numMobilePhone--;
}

MobilePhoneStart.java

    if (e.getSource() == myDelete) {
        if (inven.size() > 1) {
            inven.delete(inven.get(see));
            see--;
            if (see < 0) {
                see = 0;
            }
            showMP();
        } else {
            System.exit(0);
        }
    }

For some reason it's not creating the new list. Any ideas? Thanks in advance.

1 Answers1

3

When copying Java arrays, my preferred way is with System.arrayCopy. In your case, you have a test inside the loop which actually probably makes your approach more suitable.

Without seeing the rest of your class and how its mutable state works, it's hard to be sure about the solution. Overall, it looks OK. Possibly, there is a mistake with a reference somewhere so you don't get the thing you thought you'd got.

But one thing worth testing is that your equality test is the correct one:

if (p != list[i]) {

because you may find a ! ... .equals test works more reliably than !=.

Rick-777
  • 9,714
  • 5
  • 34
  • 50
  • I have the same result as the way I had it before. It doesn't remove the object from the array. – user3033222 Nov 29 '13 at 16:41
  • I like this answer. You should prefer `equals()` over `==`. The former is testing equality as you define it in the class definition, while the latter is testing whether you are looking at the exact same instance, which may not be what you mean here. The default `equals()` implementation just calls `equals()` on each field in an object - you may want to do more (or less). If you do write your own implementation of `equals()` on `MobilePhone`, make sure you read this link: (http://stackoverflow.com/questions/2265503/why-do-i-need-to-override-the-equals-and-hashcode-methods-in-java) – sparc_spread Nov 29 '13 at 16:42
  • This ins't the answer. It's a good practice but it didn't solve the problem. – user3033222 Nov 29 '13 at 16:44
  • How do you identify a `MobilePhone`? Is it with some kind of `id` field and nothing more? If so, you should write your own `equals()` method for `MobilePhone` that just compares `id`. Otherwise the default `equals()` methods may be comparing fields you do not care about in two `MobilePhone` instances with the same `id`, and returning `false` for equality. – sparc_spread Nov 29 '13 at 16:45
  • "For some reason it's not creating the new list" - it clearly is, but maybe it gets lost due to overwriting the wrong field or being overwritten by a subsequent alteration. Could you check how the rest of the class works, because the bug may not be in the code you listed. – Rick-777 Nov 29 '13 at 23:12