0

I am writing a large program that involves an Object called Player. The Player definition is as follows:

public class Player
{
    public static String name;
    public static Item inventory[] = new Item[10];
    public static int items;

    /* ... */

    public void addItem(String itemName, int itemType)
    {
            if ((items + 1) <= 10) {
                    inventory[items] = new Item(itemName, itemType);
                    items++;
            }
    }

    public void removeItem(int x)
    {
            for (int i = x; i < items; i++)
                    inventory[i] = inventory[i+1];
    }
}

I am adding inventory handling now because it's much easier than adding it later, but inventory isn't going to be used until much later in development. I have no way to see if removeItem works. I modified a function I wrote called strstrip to get this... Would removeItem work? If not, why?

112
  • 75
  • 11
  • 6
    if you are going to remove stuff, things like Lists would be more suitable. Typically, ArrayList. Nonetheless, no `removeItem` isn't going to work, it crashes, but you would know that if you tested it. Also, you may want to learn what `static` means – njzk2 Jun 13 '16 at 22:02
  • 1
    *I have no way to see if removeItem works*: sure you do. Add items, then remove one, then see what inventory contains what it should contain. That said, you're reinventing ArrayList. – JB Nizet Jun 13 '16 at 22:02
  • Debug is your friend! – Jorge Campos Jun 13 '16 at 22:04
  • 1
    You may want to look into unit testing. It can help with situations like this. –  Jun 13 '16 at 22:10
  • 2
    Do you know what `static` means? Do you *really* intend to have state shared between all `Player` instances? – Andy Turner Jun 13 '16 at 22:11
  • @StephenP and LinkedList needs to traverse all the nodes until the middle one, which is usually slower than shifting a contiguous block of memory. ArrayList is your best bet in nearly all situations. – JB Nizet Jun 13 '16 at 22:21
  • Is your inventory specific to each Player object ? – Learn More Jun 13 '16 at 22:24
  • No. What I disagreed about was: *If you'll be inserting in the middle of the list, LinkedList is a better choice because ArrayList must shift lots of elements to make room for the new one.* – JB Nizet Jun 13 '16 at 22:28
  • Arraylists will be going to play vital role here rather than static arrays. – Asad Ur Rehman Jun 13 '16 at 23:05

1 Answers1

1

Create unit tests for your classes especially if you going to build 'big and complicated program'. This will guarantee you that the code written will work later and if you change your code the failure of unit tests should indicate a problem. The unit test also gives you ability to check that your method works as expected.

As per other comments, consider using List interface instead of array, unless you have some specific requirement (I cannot imagine any). And definitely, having public static fields in your class looks suspicious.

EDIT

Just to indicate how code can look like and how to call methods from the main method.

public class Player {

    private String name;
    private List<Item> inventory;
    private int items;

    public Player() {
        this.inventory = new ArrayList();
    }

    public void addItem(String itemName, int itemType) {
        this.inventory.add(new Item(itemName, itemType));
    }

    public void removeItem(int x) {
        Item itemToRemove = this.inventory.get(x);
        if (itemToRemove != null) {
            this.inventory.remove(itemToRemove);
        }
    }

    public static void main(String[] args) {
        // create a new instance
        Player player = new Player();
        // call a method on the instance
        player.addItem("bla", 0);
    }
}
Mikhail Chibel
  • 1,865
  • 1
  • 22
  • 34
  • I use `static` for everything so that it can be accessed by `public static void main`. Otherwise I get an error - 'cannot access static ... from non-static context'. – 112 Jun 13 '16 at 23:28
  • Sure it will complain. The main method is static and in order to use any nonstatic methods from the class you need to create an instance of the class first. If you unsure about static... then check this post http://stackoverflow.com/questions/3903537/i-want-to-know-the-difference-between-static-method-and-non-static-method – Mikhail Chibel Jun 13 '16 at 23:31
  • what should i do to create an instance of the class - make a `public interface`? – 112 Jun 13 '16 at 23:35
  • I modified answer to indicate how to deal with the 'main' method. – Mikhail Chibel Jun 13 '16 at 23:43
  • thank you very much, that was extremely helpful! out of curiosity, why should the variables all be private - what would be the benefits? and how would I put the elements in the linked list in a specific order? – 112 Jun 13 '16 at 23:53
  • 'private' is for encapsulation and to put value into specific position you can use ' add(int index, E element)' method. – Mikhail Chibel Jun 14 '16 at 00:00