• Post Reply Bookmark Topic Watch Topic
  • New Topic

Remove method not recognizing user input in ArrayList  RSS feed

 
Christopher Sheridan
Ranch Hand
Posts: 50
IntelliJ IDE Java Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I have a Driver class, a Parent class called Plant, and three subclasses called Flower, Fungus, and Weed. The Plant class has only two attributes, ID and Name. The other classes have other attributes, but are supposed to share ID and Name with their distinct attributes. Thus far, I am able to add elements if the user chooses a Flower, and display them, but when I try to remove an element, it says it was not found in the inventory and doesn't remove it.

I feel like this is a problem with inheritance. I even tried implementing an Override method for .equals(), to no avail. I will show my Driver, Plant, and Flower class since that's as far as I've gotten. Once I can get past this problem, the other two subclasses shouldn't be a problem.

The Plant class:



The Flower class:



And finally, the Driver class. Note, there are empty blocks of code since I haven't gotten to that point yet (for example, the cases in a switch statement, the exit method, and the search/filter methods:



 
Campbell Ritchie
Marshal
Posts: 56578
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
You did something wrong with the code tags, you hade some lines too long, some excess blank lines and missed some blank lines where they ought to be. Don't worry; I have sorted those things out

Look at line 198. How does it compare with the equals() method? If there is a difference, then are you going to find the object in the list at all? Don't know. I have to go. I might be able to have another look at this thread in an hour or two.
 
Campbell Ritchie
Marshal
Posts: 56578
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Why is the hasThorns field a String rather than a boolean? You don't usually write getXXX methods to return a boolean (called predicate methods). You call them isXXX or sometimes hasXXX. I think hasHasThorns might not sound too good. What about calling the field thorny and you can have an isThorny method. Similary isPoisonous, etc.
Do fungi or weeds have thorns? If so, the thorny field should probably be in the plant class not the Fungus class. So you have repeated code in the methods to add a weed/fungus/flower. you should work out how to get rid of that repeated code. It will mean merging the three methods, or parts of them.
Don't create multiple Scanner objects pointing to System.in. You only need one, and you keep using it again and again.

Why have you got all that code in the constructor? Why have you got all those fields in the Driver class? Surely they are fields of a Plant. Your driver class (or some sort of PlantCollection class) is an appropriate place to have a List. It should of course be an instance field. Then you won't pass it as a parameter to your methods.
Why do you have a maximum size for the List? List can be any size you like, as long as you have enough memory and don't run out of indices, which means an array list cannot be larger than 2147483647 elements. (You will probably run out of memory long before you have that many elements.)
There is something wrong about your loop starting while (true). TryNow, you have a simple bit of code for the case where you want to exit:
tryAgain = false;
break;


As for the problem you are having with removal of Plant objects, please show us some details of the actual input and output. Be sure to use copy'n'paste.
You do realise you can simply call remove or removeAll or removeIf and you don't have to write your own loops. The removeIf method only works in Java8. The removeAll method would require you pass a Collection reference to it. The remove methods tend to have return values, which you can print out to see whether removal was successful.Those methods are likely to be dependent on the correct implementation of the equals() method. Did you know you should override hashCode too? But I suspect failure to override hashCode is not the cause of the current problem.
 
Christopher Sheridan
Ranch Hand
Posts: 50
IntelliJ IDE Java Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I mentioned in my original post that the thorns and smell will be booleans later. The thorn and smell are in the Flower class.

Regarding the multiple Scanners - Do I need to create a global Scanner then?

I have a size for the ArrayList because originally we were instructed to have 25 items in our inventory. I'm aware that an ArrayList can be MUCH larger than that, so I was just sticking with requirements.

The while(true) was not my code. When we first started this program, we were told to leave it alone. This current program is just expanding on previous ones. First, it was just an array, then it was an ArrayList with a Driver and Flower class. Now, it's an ArrayList with a Driver, Flower, Fungus, and Weed class.

Here is the output when I try to remove:





 
Campbell Ritchie
Marshal
Posts: 56578
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Before you try removing anything print out what you plan to remove and the inventory List. See whether you can see the equality.
The try iterating the List and comparing each element with what you are trying to remove, using its equals method. I still think there is something wrong with the equals methods.

Did you know that you cannot add fields to a class with an equals method without violating the Liskov substitution principle? I think you should put all the fields into the Plant class. Also the equals method. Then you can have very simple subclasses:-And that is it. 8 lines. You will obviously have to expand that Plant't equals hashCode and toString methods.
 
Christopher Sheridan
Ranch Hand
Posts: 50
IntelliJ IDE Java Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I don't know how I would do that, because:

Plant is supposed to have ID and name.

Flower is supposed to have color, scent, and thorny.

Fungus is supposed to have color and poisonous.

Weed is supposed to have color, edible, and medicinal.

I know some of these are going to be booleans later on, but just for now they are strings.
 
Campbell Ritchie
Marshal
Posts: 56578
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
There is one way to get all those fields in use, but it isn't beginner's stuff, and it isn't easy but it gets easier with practice.
  • 1: Put all the Plant classes into a package by themselves.
  • 2: Put all those fields in the Plant class with private access and default values and getXXX/isXXX methods as appropriate.
  • 3: Give all those methods package-private (=default) access. Easy: don't write an access modifier.
  • 4: Any class needing access to a field, override the getXXX method with public access.
  • 5: All other classes go in a different package and import the Plant classes.
  • 6: You can now put the equals() and hashCode() methods in the Plant class.
  • That overriding of a getXXX method is easy:-The getXXX method in the superclass would look like any get method, but without an access modifier. If you miss out the isThorny method in the Fungus class, it will appear that fungi don't have thorns. You can fiddle the constructor parameters:-You pass 5 arguments to that constructor but you actually pass 6 arguments to the superclass' constructor; the false represents thorny. Because the fields are private and the getXXX methods have package‑private access, no code outwith that package will ever be aware of those data for a fungus.
    Another complicated way to do things: find out about factory methods. Once you have used factory methods a few times, they become easier to write.

    Doubtless somebody else will think of a better way to do all that.

    Actually you can have scented fungi(e.g. Phallus impudicus), and poisonous fungi (e.g. Amanita phalloides) and medicinal fungi (e.g. Penicillium notatum). You can have poisonous flowers (e.g. Digitalis purpurea = foxglove) and medicinal flowers (e.g. Digitalis purpurea).
     
    Christopher Sheridan
    Ranch Hand
    Posts: 50
    IntelliJ IDE Java Linux
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Lol. I mean for this particular program, that's all I'm required to do (regarding poisonous, medicinal, etc.).

    And the separate package stuff is over my head. I'm barely through my second semester of programming. This is getting deeper than what it should be, and surely it is something simple that I'm missing.
     
    Paul Clapham
    Sheriff
    Posts: 22835
    43
    Eclipse IDE Firefox Browser MySQL Database
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    I don't really follow the design of your code for removing a flower. First you ask for the ID number of the plant to remove, which should be sufficient to identify the plant. But then you go on to ask the user to enter everything else about the plant, and if they don't spell everything exactly, precisely, the same as the data on file for that plant then it doesn't get deleted.

    After all the ID number should be a unique key, shouldn't it? For that reason your Flower.equals() method isn't right -- it should just compare the ID numbers of the two Flower objects, and the two are equal if, and only if, the ID numbers are the same. You don't mean to allow two different flowers with the same ID number but different thorniness attributes, right?
     
    Christopher Sheridan
    Ranch Hand
    Posts: 50
    IntelliJ IDE Java Linux
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    You made a valid point there. I changed it to where it just asks for the ID number and name. I can probably make it even shorter with your suggestion of just the ID, so I might do that.

    I also took the suggestion of using just one Scanner instead of making new ones every time I take input, so I made it global.

    What I DON'T understand, however, is why (even if it's not needed in this program) when I ask a user to input all of the attributes of a plant (let's say they want to remove a flower, and enter the ID, name, color, etc), the getColor, getSmell, etc are not allowed in the method. They show up red in my code. I'm still learning inheritance and polymorphism, so there's surely something I'm missing. It doesn't really matter for this particular program since I can just use ID or ID and name, but I would still like to know.

    One last question. This program is expanding on a previous program. The primary difference is that the previous program only had one class, which was Flower. I used .equals() for the remove method and it worked fine. Why couldn't I use it for this program? What made it so different? It only had two additional classes.

    By the way, I finally got it to work last night. I went with an Iterator and it worked perfectly. See below:

     
    Campbell Ritchie
    Marshal
    Posts: 56578
    172
    • Likes 1
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    I have already told you. There is something wrong with the equals() method. Go through your list (as I told you earlier) and see how many matches you get with the equals() method. If the equals() method returns false, the Lost will interpret that as a different unrelated object and won't remove it. You can remove items with a particular ID field, but not with the equals() method. You can use the removeIf method but that only works in Java8.
     
    Campbell Ritchie
    Marshal
    Posts: 56578
    172
    • Likes 1
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    If the equals() method compares n fields then you mist pass all those n fields for the equals() method to recognise them. You should have a constructor which takes a certain number of parameters so you can only create an instance if you pass all arguments.
     
    Christopher Sheridan
    Ranch Hand
    Posts: 50
    IntelliJ IDE Java Linux
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Update:

    Fixed problem where smell, thorns, edible, poisonous, and medicine were Strings rather than booleans. Changed all instances of input.nextLine() to input.nextBoolean(), and variables in constructors where needed. I researched the logic of vocabulary and booleans, and discovered that any time the words "is", "has", etc. are used, booleans should be implemented.

    Fixed problem with loop iterations to remove an object from ArrayList using Iterator.

    If I missed anything else, my apologies. Anyway, here is my new working remove method. Thank you all for your help and suggestions:





     
    Campbell Ritchie
    Marshal
    Posts: 56578
    172
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Christopher Sheridan wrote:. . . Fixed problem where smell, thorns, edible, poisonous, and medicine were Strings rather than booleans. . . .
    Getting rid of the Strings was a good idea. But that method look inordinately long and complicated. Why don't you simply use the remove method of the List interface? Why are you separating flower from weed from fungus? You are asking for ID and name in all three cases. Why are you repeating the removal code?
    If you simply call remove(), doesn't it return the item removed? Then you can simply print what was removed. And what about the removeIf method (only works in Java8)?
     
    Campbell Ritchie
    Marshal
    Posts: 56578
    172
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    A few minutes ago, I wrote:. . . If you simply call remove(), doesn't it return the item removed? . . .
    No it doesn't. It returns true or false. But you can still use that to print what was removed.
     
    • Post Reply Bookmark Topic Watch Topic
    • New Topic
    Boost this thread!