Win a copy of Building Blockchain Apps this week in the Cloud/Virtualization forum!
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Paul Clapham
  • Liutauras Vilda
  • Knute Snortum
  • Bear Bibeault
Sheriffs:
  • Devaka Cooray
  • Jeanne Boyarsky
  • Junilu Lacar
Saloon Keepers:
  • Ron McLeod
  • Stephan van Hulst
  • Tim Moores
  • Carey Brown
  • salvin francis
Bartenders:
  • Tim Holloway
  • Piet Souris
  • Frits Walraven

Problem removing an element from a List

 
Ranch Hand
Posts: 90
2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
problem in the remove method.
the element should be the one removed, not the name.
The name needs to match the element, for the element to be removed if found in the list

sugestion for the element name?
 
Marshal
Posts: 68115
258
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
You should have opened a new th‍read for that removal question. Please go through the possible paths of execution in your suggestion, where you will find one line is never reached. Please work out when you will reach the line with the exception in.
Don't throw plain simple Exception if possible. Throw a more specific exception. But why are you throwing an exception in the first place? There is nothing abnormal about not finding a specific name, so why should it need an exception?
Why don't you use a built‑in method, like this one?
There is something wrong with your variable names if you can write name.getName().
 
Campbell Ritchie
Marshal
Posts: 68115
258
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Th‍read split from this th‍read.
 
Lg Long
Ranch Hand
Posts: 90
2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Indeed the else is not reached. Can't really explain why the exception in the else block is not reached. Need to think more

I just want to let the user know, there are no more birds. Indeed there is nothing wrong.
I could just throw a message.
But couldn't figure it out.

 
Ranch Foreman
Posts: 129
11
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I don't think Java likes it when you try to remove an entry from a List inside a For Each loop. It's considered concurrent modification of the List and is not allowed.

I recommend that you try an Iterator on the list. Loop through using iterator.next and hasNext and then do remove when you find your desired entry to remove.

Also don't have an empty catch block. It makes debugging horrible since your code will fail silently and you'll wonder why it's not doing anything and also not showing any errors. At minimum, do ex.printStackTrace() in the catch.
 
Lg Long
Ranch Hand
Posts: 90
2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thank you. I didn't want to have nothing in the catch bloc, didn't know about the e.printStackTrace();

That is why I use the boolean, I overcome the Concurrent Exception.
I do plan on using the Iterator on the refractor, for now I will keep the for loop.
Very good advice and reason to use the Iterator, couldn't really find a reason why.
 
Campbell Ritchie
Marshal
Posts: 68115
258
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I still think I prefer the methods of the List itself. I had forgotten that the for‑each loop would count as concurrent modification.
 
Zachary Griggs
Ranch Foreman
Posts: 129
11
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hmm, I still think there are some issues with the code provided:


intemFound boolean is specified, but never set to anything. The code doesn't set it to true, so that means it's going to be always be false and your !intemFound will always result in true. I think you meant to set the boolean inside the if.

Your try/catch only surrounds this code:
           if(!intemFound){
               System.out.println(name + " no longer for sale. Chose another option");
           }
That code above can't ever actually throw an Exception. I think you meant to put the try block around the code above this.

Also I would rename it to "itemFound" to correct the spelling.

The cleanest way to fix this code is to implement the equals() method on the BaseProduct class. Then you are able to make this line just a single call into the List and it'll find and remove it for you. A less clean (but also functional) way is to use an iterator. If you don't want to do either right now, you can improve on your current implementation like this (pseudocode):

- Set integer index = -1
- Loop through for currentIndex -> list.length
- - Get the item, compare the name.
- - If name is the same, set index = currentIndex
- After loop, if index >= 0, list.remove(index)
- Else print message saying it didn't exist
 
Bartender
Posts: 3777
154
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
A List has a method 'removeIf' that can make life easy. See the API of the Collection class.
 
Lg Long
Ranch Hand
Posts: 90
2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
This is my Iterator attempt
I get the concurrent exception

 
Marshal
Posts: 6869
182
Eclipse IDE Postgres Database VI Editor Chrome Java Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
In the second method, why do you have the return outside of the if braces?  How many times will the for-loop iterate?
 
Lg Long
Ranch Hand
Posts: 90
2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Nice one Piet!
still don't know how to wrap that in a condition so it will print a message if not name not found




The return outside the if will make the for loop iterate twice, but if the condition is met, should exit.
Did I get it right?
 
Lg Long
Ranch Hand
Posts: 90
2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
So, if I understand this:
The for loop takes the element and is looping with it through the list. When the condition is meet, the statement is executed and it ends.
If the statement is not executed, the message is printed.
Correct?

 
Campbell Ritchie
Marshal
Posts: 68115
258
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Your code still looks as if you had been guessing. Please start by explaining to us, and to yourself, what you are trying to do. Are you trying to remove all elements with a particular attribute or the first? Do the objects you are putting into that List have a correct equals() and hashCode() methods? The only attempt I think any good is when Piet persuaded you to try removeIf().
 
Lg Long
Ranch Hand
Posts: 90
2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
ok, thank you for asking.

The point of removeFromList method is to remove an element, 1 at a time.
If I attempt to remove an element of specified name and the name does not match the element, the message 'not found' is printed.
Indeed it seams like guessing, because I don't think i understand how forLoops work entirely.
That is what i was trying to gain through my explanation.
Did I explain correctly?

The methods equals() and hashCode() are generated properly in BaseProduct.

I don't see how I can wrap Piet's solution so I can get a message 'not found' in case the condition is not meet.
this is my attempt

 
Marshal
Posts: 25215
65
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Lg Long wrote:I don't see how I can wrap Piet's solution so I can get a message 'not found' in case the condition is not meet.



1. Note the size of the list.

2. Remove the list entries matching the predicate.

3. Compare the current size of the list to the number you noted earlier.

4. Do what you have to do.
 
Campbell Ritchie
Marshal
Posts: 68115
258
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
So you want to remove one element where the names are the same. And if nothing is removed, print, “Not found.”
As you know, the method I first gave you a link to, which happens to be removeIf(), removes all elements for which a predicate applies. So that may not be what you want. Both removeIf() and for‑each use an Iterator in the background, so they are prone to concurrent modification problems. Iterators are specifically designed not to allow modification of the collection they are iterating by anything else. So, let's think about ways of removing one element. I can think of about three just at the moment, but there are bound to be a lot more:-
  • 1: Use a for loop, but add a second continuation condition to loop only until an element has been removed.
  • 2: If we are iterating on a condition, maybe we are using sentinel controlled repetition rather than counter controlled. So let's try a while loop.
  • 3: Lists have methods for finding elements. Use them.
  • No 3 looks so much neater and cleaner, but it depends on:-
  • i: Can you create such a temporary object to remove?
  • ii: Does that temporary object have correctly overridden equals() and hashCode() methods. You probably should always override those two methods if you are putting objects into Lists, otherwise you risk not finding the object you want.
  • The get() method executes in constant time (very fast) on an ArrayList, but not at all fast (linear time) on a linked list. For that you should use the Iterator, which will work on an array list too:-It is customary to put Iterators into while loops, but the for loop version has the advantage that the Iterator goes out of scope sooner:-Note both examples with while have additional {} to restrict the scope of a variable. Change XYZ to the real name of your class, probably BaseProduct.
     
    Paul Clapham
    Marshal
    Posts: 25215
    65
    Eclipse IDE Firefox Browser MySQL Database
    • Likes 1
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    I know people have invested a lot of time and effort into this post, but could I just ask a stupid question:

    Is it important for these things to be stored in a List? If they were stored in a Set, then removing an element would be a simple one-liner.

    (And thanks for removeIf, Piet, I just made a chunk of my code much more comprehensible.)
     
    Lg Long
    Ranch Hand
    Posts: 90
    2
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hi Campbell,

    Thank you for taking the time to help. I have some things to do and I might not get back on the post till Wednesday.
    I will study your suggestions furthermore.
    I like your approach in the first loop, there are 2 conditions, which I read about but did not practice, so couldn't think of.
    The fifth suggestion line 4, bracket after column. The while loop doesn't behave as expected. I'll have to see how to fix that.

    Paul,
    You are right, would be easier to use sets, but we need a data structure that would allow duplicate objects.
    We did not reach the point where the quantity would be a value in the object.

    For further reading on how this thread started, please read the description of the initial problem here: coderanch.com/p/3384473

    Besides having a field in the BaseProduct, of name quantity, do you see another way of using sets, since at the moment we only add objects to the List.
    I will be continuing this thread from Wednesday on.
    Thank you CodeRanch, I think I found my mentor here!
     
    Campbell Ritchie
    Marshal
    Posts: 68115
    258
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator

    Lg Long wrote:. . . Thank you for taking the time to help. . . .

    That's a pleasure

    The fifth suggestion line 4, bracket after column.

    That's because I mistakenly put a semicolon in somewhere. Sorry.

    The while loop doesn't behave as expected. I'll have to see how to fix that. . . .

    Please explain more about that whenever you are back here.
     
    Paul Clapham
    Marshal
    Posts: 25215
    65
    Eclipse IDE Firefox Browser MySQL Database
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator

    Lg Long wrote:Paul,
    You are right, would be easier to use sets, but we need a data structure that would allow duplicate objects.



    Fair enough. It's just that it looked like you were trying to write code which would only remove one object, given a key.
     
    Piet Souris
    Bartender
    Posts: 3777
    154
    • Likes 1
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    @Paul
    you're welcome!

    @Lg Long

    just for the sake of having a different way, not because it is better: how about using a Map? A Map<name-in-lowercase (String), List<BaseProduct>>. It is mostly good for rehearsing some map-techniques.

    Having such a Map, you can have methods like:

    As said, I prefer the simplicity of a List and an Iterator, but this should be a nice practising.
     
    Lg Long
    Ranch Hand
    Posts: 90
    2
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hi Campbell Ritchie:

    Sugestion 1:

    Line 4: missing .getName()


    Sugestion 2:

    Line 6 missing .getName()


    Suggestion 5: line 5 is missing .getName()



    Suggestion 6: line 6 is missing .getName



    For the conditions to work equals() and hashCode() methods need to be overridden.

     
    If you settle for what they are giving you, you deserve what you get. Fight for this tiny ad!
    Java file APIs (DOC, XLS, PDF, and many more)
    https://products.aspose.com/total/java
    • Post Reply Bookmark Topic Watch Topic
    • New Topic
    Boost this thread!