Character class with name, gender, and inventory list

The name of the pictureThe name of the pictureThe name of the pictureClash Royale CLAN TAG#URR8PPP





.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;







up vote
3
down vote

favorite












I am trying to create a class to test my understanding, and it takes an ArrayList as parameter. The decision of choosing an ArrayList is because it does not have a fixed size and I can add more elements to it later.



I came out with this implementation:



public class Character 

private String name;
private char gender;
private List<String> inventory = new ArrayList<String>();

public Character(String name, char gender, List<String> inventory)
this.name = name;
this.gender = gender;
this.inventory.addAll(inventory); // Is this a good idea?


public String getName()
return name;


public char getGender()
return gender;


public List<String> getInventory()
return inventory;





And in the main class:



public class Main 

public static void main(String args)

Character coolKid = new Character("Cool Kid", 'f',
Arrays.asList("just", "testing"));

System.out.println(coolKid.getInventory());
// returns [just, testing]





My main concern is "dynamically" adding elements to the ArrayList when instantiating, as well as using addAll in the constructor, and List as the type for the inventory argument in the constructor.



Is there a better way to accomplish this?







share|improve this question



























    up vote
    3
    down vote

    favorite












    I am trying to create a class to test my understanding, and it takes an ArrayList as parameter. The decision of choosing an ArrayList is because it does not have a fixed size and I can add more elements to it later.



    I came out with this implementation:



    public class Character 

    private String name;
    private char gender;
    private List<String> inventory = new ArrayList<String>();

    public Character(String name, char gender, List<String> inventory)
    this.name = name;
    this.gender = gender;
    this.inventory.addAll(inventory); // Is this a good idea?


    public String getName()
    return name;


    public char getGender()
    return gender;


    public List<String> getInventory()
    return inventory;





    And in the main class:



    public class Main 

    public static void main(String args)

    Character coolKid = new Character("Cool Kid", 'f',
    Arrays.asList("just", "testing"));

    System.out.println(coolKid.getInventory());
    // returns [just, testing]





    My main concern is "dynamically" adding elements to the ArrayList when instantiating, as well as using addAll in the constructor, and List as the type for the inventory argument in the constructor.



    Is there a better way to accomplish this?







    share|improve this question























      up vote
      3
      down vote

      favorite









      up vote
      3
      down vote

      favorite











      I am trying to create a class to test my understanding, and it takes an ArrayList as parameter. The decision of choosing an ArrayList is because it does not have a fixed size and I can add more elements to it later.



      I came out with this implementation:



      public class Character 

      private String name;
      private char gender;
      private List<String> inventory = new ArrayList<String>();

      public Character(String name, char gender, List<String> inventory)
      this.name = name;
      this.gender = gender;
      this.inventory.addAll(inventory); // Is this a good idea?


      public String getName()
      return name;


      public char getGender()
      return gender;


      public List<String> getInventory()
      return inventory;





      And in the main class:



      public class Main 

      public static void main(String args)

      Character coolKid = new Character("Cool Kid", 'f',
      Arrays.asList("just", "testing"));

      System.out.println(coolKid.getInventory());
      // returns [just, testing]





      My main concern is "dynamically" adding elements to the ArrayList when instantiating, as well as using addAll in the constructor, and List as the type for the inventory argument in the constructor.



      Is there a better way to accomplish this?







      share|improve this question













      I am trying to create a class to test my understanding, and it takes an ArrayList as parameter. The decision of choosing an ArrayList is because it does not have a fixed size and I can add more elements to it later.



      I came out with this implementation:



      public class Character 

      private String name;
      private char gender;
      private List<String> inventory = new ArrayList<String>();

      public Character(String name, char gender, List<String> inventory)
      this.name = name;
      this.gender = gender;
      this.inventory.addAll(inventory); // Is this a good idea?


      public String getName()
      return name;


      public char getGender()
      return gender;


      public List<String> getInventory()
      return inventory;





      And in the main class:



      public class Main 

      public static void main(String args)

      Character coolKid = new Character("Cool Kid", 'f',
      Arrays.asList("just", "testing"));

      System.out.println(coolKid.getInventory());
      // returns [just, testing]





      My main concern is "dynamically" adding elements to the ArrayList when instantiating, as well as using addAll in the constructor, and List as the type for the inventory argument in the constructor.



      Is there a better way to accomplish this?









      share|improve this question












      share|improve this question




      share|improve this question








      edited Feb 26 at 4:58









      200_success

      123k14142399




      123k14142399









      asked Feb 25 at 14:23









      ManuAlvarado22

      694




      694




















          1 Answer
          1






          active

          oldest

          votes

















          up vote
          4
          down vote













          This is a very straightforward implementation. Not much to improve.



          Especially copying the list passed in from the caller is a good safety measure, as it makes sure that, if the caller later modifies his list, this doesn't affect the Character's inventory. It's always a good idea that changing an instance's state is done by using its methods (so it can adjust itself appropriately, e.g. drop some items if the inventory becomes too heavy).



          A few ideas to think about:



          Similarly to the constructor, you can make the getInventory() method safe against a caller's attempts to modify the list, by encapsulating it in a read-only wrapper:



          return Collections.unmodifiableList(inventory);


          Then the caller can read the list, but not change it.



          You might want to implement some more inventory methods to add, remove or find inventory items.



          There also is a programming style where you wouldn't implement add, remove or find methods for the inventory, and have the getInventory() method return the internal list, allowing the caller to directly modify that list. But that breaks encapsulation and generally leads to quality problems, so don't do it that way.



          Probably, the ordering of items in the inventory doesn't matter, so you could use a Set instead of a List (but then you can't have multiple instances of the same item).



          And you could change the constructor parameter inventory from List to Collection, allowing the caller to pass the items as List or Set, whatever suits him better.



          Maybe, when your project evolves, you'll find that inventory items could have their own properties and behaviour, so you might want to change from List<String> to List<InventoryItem> early.






          share|improve this answer























          • Thank you for you answer! I am quite new to OOP, and specially to Java. What is the benefit of encapsulating the inventory getter? Also, today i realized i could prefer a dictionary, or Python's dictionary equivalent in Java, for handling inventory, since it would be a good idea to classify items by type. I am going to definitely change the type to Collection, it was a doubt i had. Still confused with types. Excellent, i am sure i will create an InventoryItem class as well.
            – ManuAlvarado22
            Feb 26 at 10:11











          • @ManuAlvarado22 I added a few words on encapsulation.
            – Ralf Kleberhoff
            Feb 26 at 11:00










          Your Answer




          StackExchange.ifUsing("editor", function ()
          return StackExchange.using("mathjaxEditing", function ()
          StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
          StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
          );
          );
          , "mathjax-editing");

          StackExchange.ifUsing("editor", function ()
          StackExchange.using("externalEditor", function ()
          StackExchange.using("snippets", function ()
          StackExchange.snippets.init();
          );
          );
          , "code-snippets");

          StackExchange.ready(function()
          var channelOptions =
          tags: "".split(" "),
          id: "196"
          ;
          initTagRenderer("".split(" "), "".split(" "), channelOptions);

          StackExchange.using("externalEditor", function()
          // Have to fire editor after snippets, if snippets enabled
          if (StackExchange.settings.snippets.snippetsEnabled)
          StackExchange.using("snippets", function()
          createEditor();
          );

          else
          createEditor();

          );

          function createEditor()
          StackExchange.prepareEditor(
          heartbeatType: 'answer',
          convertImagesToLinks: false,
          noModals: false,
          showLowRepImageUploadWarning: true,
          reputationToPostImages: null,
          bindNavPrevention: true,
          postfix: "",
          onDemand: true,
          discardSelector: ".discard-answer"
          ,immediatelyShowMarkdownHelp:true
          );



          );








           

          draft saved


          draft discarded


















          StackExchange.ready(
          function ()
          StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f188323%2fcharacter-class-with-name-gender-and-inventory-list%23new-answer', 'question_page');

          );

          Post as a guest






























          1 Answer
          1






          active

          oldest

          votes








          1 Answer
          1






          active

          oldest

          votes









          active

          oldest

          votes






          active

          oldest

          votes








          up vote
          4
          down vote













          This is a very straightforward implementation. Not much to improve.



          Especially copying the list passed in from the caller is a good safety measure, as it makes sure that, if the caller later modifies his list, this doesn't affect the Character's inventory. It's always a good idea that changing an instance's state is done by using its methods (so it can adjust itself appropriately, e.g. drop some items if the inventory becomes too heavy).



          A few ideas to think about:



          Similarly to the constructor, you can make the getInventory() method safe against a caller's attempts to modify the list, by encapsulating it in a read-only wrapper:



          return Collections.unmodifiableList(inventory);


          Then the caller can read the list, but not change it.



          You might want to implement some more inventory methods to add, remove or find inventory items.



          There also is a programming style where you wouldn't implement add, remove or find methods for the inventory, and have the getInventory() method return the internal list, allowing the caller to directly modify that list. But that breaks encapsulation and generally leads to quality problems, so don't do it that way.



          Probably, the ordering of items in the inventory doesn't matter, so you could use a Set instead of a List (but then you can't have multiple instances of the same item).



          And you could change the constructor parameter inventory from List to Collection, allowing the caller to pass the items as List or Set, whatever suits him better.



          Maybe, when your project evolves, you'll find that inventory items could have their own properties and behaviour, so you might want to change from List<String> to List<InventoryItem> early.






          share|improve this answer























          • Thank you for you answer! I am quite new to OOP, and specially to Java. What is the benefit of encapsulating the inventory getter? Also, today i realized i could prefer a dictionary, or Python's dictionary equivalent in Java, for handling inventory, since it would be a good idea to classify items by type. I am going to definitely change the type to Collection, it was a doubt i had. Still confused with types. Excellent, i am sure i will create an InventoryItem class as well.
            – ManuAlvarado22
            Feb 26 at 10:11











          • @ManuAlvarado22 I added a few words on encapsulation.
            – Ralf Kleberhoff
            Feb 26 at 11:00














          up vote
          4
          down vote













          This is a very straightforward implementation. Not much to improve.



          Especially copying the list passed in from the caller is a good safety measure, as it makes sure that, if the caller later modifies his list, this doesn't affect the Character's inventory. It's always a good idea that changing an instance's state is done by using its methods (so it can adjust itself appropriately, e.g. drop some items if the inventory becomes too heavy).



          A few ideas to think about:



          Similarly to the constructor, you can make the getInventory() method safe against a caller's attempts to modify the list, by encapsulating it in a read-only wrapper:



          return Collections.unmodifiableList(inventory);


          Then the caller can read the list, but not change it.



          You might want to implement some more inventory methods to add, remove or find inventory items.



          There also is a programming style where you wouldn't implement add, remove or find methods for the inventory, and have the getInventory() method return the internal list, allowing the caller to directly modify that list. But that breaks encapsulation and generally leads to quality problems, so don't do it that way.



          Probably, the ordering of items in the inventory doesn't matter, so you could use a Set instead of a List (but then you can't have multiple instances of the same item).



          And you could change the constructor parameter inventory from List to Collection, allowing the caller to pass the items as List or Set, whatever suits him better.



          Maybe, when your project evolves, you'll find that inventory items could have their own properties and behaviour, so you might want to change from List<String> to List<InventoryItem> early.






          share|improve this answer























          • Thank you for you answer! I am quite new to OOP, and specially to Java. What is the benefit of encapsulating the inventory getter? Also, today i realized i could prefer a dictionary, or Python's dictionary equivalent in Java, for handling inventory, since it would be a good idea to classify items by type. I am going to definitely change the type to Collection, it was a doubt i had. Still confused with types. Excellent, i am sure i will create an InventoryItem class as well.
            – ManuAlvarado22
            Feb 26 at 10:11











          • @ManuAlvarado22 I added a few words on encapsulation.
            – Ralf Kleberhoff
            Feb 26 at 11:00












          up vote
          4
          down vote










          up vote
          4
          down vote









          This is a very straightforward implementation. Not much to improve.



          Especially copying the list passed in from the caller is a good safety measure, as it makes sure that, if the caller later modifies his list, this doesn't affect the Character's inventory. It's always a good idea that changing an instance's state is done by using its methods (so it can adjust itself appropriately, e.g. drop some items if the inventory becomes too heavy).



          A few ideas to think about:



          Similarly to the constructor, you can make the getInventory() method safe against a caller's attempts to modify the list, by encapsulating it in a read-only wrapper:



          return Collections.unmodifiableList(inventory);


          Then the caller can read the list, but not change it.



          You might want to implement some more inventory methods to add, remove or find inventory items.



          There also is a programming style where you wouldn't implement add, remove or find methods for the inventory, and have the getInventory() method return the internal list, allowing the caller to directly modify that list. But that breaks encapsulation and generally leads to quality problems, so don't do it that way.



          Probably, the ordering of items in the inventory doesn't matter, so you could use a Set instead of a List (but then you can't have multiple instances of the same item).



          And you could change the constructor parameter inventory from List to Collection, allowing the caller to pass the items as List or Set, whatever suits him better.



          Maybe, when your project evolves, you'll find that inventory items could have their own properties and behaviour, so you might want to change from List<String> to List<InventoryItem> early.






          share|improve this answer















          This is a very straightforward implementation. Not much to improve.



          Especially copying the list passed in from the caller is a good safety measure, as it makes sure that, if the caller later modifies his list, this doesn't affect the Character's inventory. It's always a good idea that changing an instance's state is done by using its methods (so it can adjust itself appropriately, e.g. drop some items if the inventory becomes too heavy).



          A few ideas to think about:



          Similarly to the constructor, you can make the getInventory() method safe against a caller's attempts to modify the list, by encapsulating it in a read-only wrapper:



          return Collections.unmodifiableList(inventory);


          Then the caller can read the list, but not change it.



          You might want to implement some more inventory methods to add, remove or find inventory items.



          There also is a programming style where you wouldn't implement add, remove or find methods for the inventory, and have the getInventory() method return the internal list, allowing the caller to directly modify that list. But that breaks encapsulation and generally leads to quality problems, so don't do it that way.



          Probably, the ordering of items in the inventory doesn't matter, so you could use a Set instead of a List (but then you can't have multiple instances of the same item).



          And you could change the constructor parameter inventory from List to Collection, allowing the caller to pass the items as List or Set, whatever suits him better.



          Maybe, when your project evolves, you'll find that inventory items could have their own properties and behaviour, so you might want to change from List<String> to List<InventoryItem> early.







          share|improve this answer















          share|improve this answer



          share|improve this answer








          edited Feb 26 at 10:59


























          answered Feb 25 at 18:58









          Ralf Kleberhoff

          70527




          70527











          • Thank you for you answer! I am quite new to OOP, and specially to Java. What is the benefit of encapsulating the inventory getter? Also, today i realized i could prefer a dictionary, or Python's dictionary equivalent in Java, for handling inventory, since it would be a good idea to classify items by type. I am going to definitely change the type to Collection, it was a doubt i had. Still confused with types. Excellent, i am sure i will create an InventoryItem class as well.
            – ManuAlvarado22
            Feb 26 at 10:11











          • @ManuAlvarado22 I added a few words on encapsulation.
            – Ralf Kleberhoff
            Feb 26 at 11:00
















          • Thank you for you answer! I am quite new to OOP, and specially to Java. What is the benefit of encapsulating the inventory getter? Also, today i realized i could prefer a dictionary, or Python's dictionary equivalent in Java, for handling inventory, since it would be a good idea to classify items by type. I am going to definitely change the type to Collection, it was a doubt i had. Still confused with types. Excellent, i am sure i will create an InventoryItem class as well.
            – ManuAlvarado22
            Feb 26 at 10:11











          • @ManuAlvarado22 I added a few words on encapsulation.
            – Ralf Kleberhoff
            Feb 26 at 11:00















          Thank you for you answer! I am quite new to OOP, and specially to Java. What is the benefit of encapsulating the inventory getter? Also, today i realized i could prefer a dictionary, or Python's dictionary equivalent in Java, for handling inventory, since it would be a good idea to classify items by type. I am going to definitely change the type to Collection, it was a doubt i had. Still confused with types. Excellent, i am sure i will create an InventoryItem class as well.
          – ManuAlvarado22
          Feb 26 at 10:11





          Thank you for you answer! I am quite new to OOP, and specially to Java. What is the benefit of encapsulating the inventory getter? Also, today i realized i could prefer a dictionary, or Python's dictionary equivalent in Java, for handling inventory, since it would be a good idea to classify items by type. I am going to definitely change the type to Collection, it was a doubt i had. Still confused with types. Excellent, i am sure i will create an InventoryItem class as well.
          – ManuAlvarado22
          Feb 26 at 10:11













          @ManuAlvarado22 I added a few words on encapsulation.
          – Ralf Kleberhoff
          Feb 26 at 11:00




          @ManuAlvarado22 I added a few words on encapsulation.
          – Ralf Kleberhoff
          Feb 26 at 11:00












           

          draft saved


          draft discarded


























           


          draft saved


          draft discarded














          StackExchange.ready(
          function ()
          StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f188323%2fcharacter-class-with-name-gender-and-inventory-list%23new-answer', 'question_page');

          );

          Post as a guest













































































          Popular posts from this blog

          Greedy Best First Search implementation in Rust

          Function to Return a JSON Like Objects Using VBA Collections and Arrays

          C++11 CLH Lock Implementation