This week's book giveaway is in the Agile and Other Processes forum.
We're giving away four copies of The Little Book of Impediments (e-book only) and have Tom Perry on-line!
See this thread for details.
Win a copy of The Little Book of Impediments (e-book only) this week in the Agile and Other Processes forum!
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

Move in the "array list" with "Next" and "Previous" buttons

 
Christian Klugesherz
Greenhorn
Pie
Posts: 26
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hello All,

I'm beginner in Java.
There is now some days that I try to do something which should normally be really simple. :-(
A tried several methods, but I didn't success for now.
I don't know if it is the best approach, so please help.

Goal
  Class: "Person" with 2 methods for "name" and "age"
  Move in the "array list" of "Person" with "Next" and "Previous" buttons.
"New" and "Delete" buttons for add or remove a "Person"
"Save" to Modify the name of   Person.
I tested several approach, but didn't succeed

Below the code who is generating an error.
Sorry for that, but as I have read that the best approach is to use iterators, I preferred to start with that

Thanks for your help.

Christian

File: Person.java


File : MoveArrayList


 
Junilu Lacar
Bartender
Pie
Posts: 8790
81
Android Eclipse IDE IntelliJ IDE Java Linux Mac Scala Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
First, you should follow standard naming conventions for variables and fields. Right now, your field names are capitalized; they should start with lowercase. Your names make your code confusing to read because people who are used to the standard convention will think that those field names refer to class names, which always start with a capital letter.

This code:
should be changed to this:

Notice also that on line 19, the declared type should be the interface type List, not the implementation type ArrayList.

Next, please provide more details about the error that you are getting. It's not enough to just say "This is the code that is generating an error..."; you have to tell us exactly what error you're getting; you need to TellTheDetails.

Copy/paste the exact error message that you are seeing.
 
Junilu Lacar
Bartender
Pie
Posts: 8790
81
Android Eclipse IDE IntelliJ IDE Java Linux Mac Scala Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
and Welcome to the Ranch!
 
Campbell Ritchie
Sheriff
Posts: 51368
87
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Welcome to the Ranch

You seem to have an Iterator not used as a variable local to a loop. That isn't right. You are liable to exceptions like that. Only use Iterators as local loop variables. Use an index for the List and its add(int, E) method to add elements. You must tell us what the error is if we are to help.
 
Junilu Lacar
Bartender
Pie
Posts: 8790
81
Android Eclipse IDE IntelliJ IDE Java Linux Mac Scala Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Campbell Ritchie wrote:You seem to have an Iterator not used as a variable local to a loop. That isn't right. You are liable to exceptions like that. Only use Iterators as local loop variables. Use an index for the List and its add(int, E) method to add elements.

I thought the same thing at first. However, if you go over all the event listeners that OP has created for new, delete, next, previous, he consistently uses the ListIterator basically as a cursor into his list.  Adding any code that will change the list via any method other than the ListIterator methods will cause the ListIterator to fail-fast. As such, I would caution OP against just switching to direct access of the list, if you want to go that way. I think it's ok to stick with the ListIterator for now but just make sure you're consistently doing so throughout your code.

That said, I do agree with Campbell that an Iterator or ListIterator is something that you would normally only want to use in a very limited scope like as a local variable in a method.

There are is a dangerous problem with how OP is using the ListIterator, exemplified by the following snippet:

The above section of code calls the ListIterator.next() without checking for ListIterator.hasNext() first. This runs the risk of getting a NoSuchElementException thrown in your face, which is most likely the error that OP is seeing. The problem is replicated in a few places in the code.  The same kind of problem exists with OP's use of the previous() method, with no preceding check for previousIndex().
 
Christian Klugesherz
Greenhorn
Pie
Posts: 26
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hello All,

Thank you for your prompt reaction and help. :-)
I will try to compile all this together and will come back

If I understood:
To use variable iterator as local variables
To work with the list, either that with iterator

Jus something that I didn't undrstood.
Notice also that on line 19, the declared type should be the interface type List, not the implementation type ArrayList.

What did I wrong ?


Thanks

Christian
 
Campbell Ritchie
Sheriff
Posts: 51368
87
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Yes, maybe I was a bit too peremptory about that iterator. But I still think it is simpler and safer to use a number as the index. Another thing: you shou‍ld not store the List as part of a display class. You are probably all right here because the class isn't really a display class, but in larger applications you shou‍ld use separate classes, in which case you would probably want an interface with methods likeWith a boolean return type, you can use the methods to test whether the move is successful and then add:-
 
Christian Klugesherz
Greenhorn
Pie
Posts: 26
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Please to consider here as a simple example, where the goal is just to walk through the list with buttons, and add or remove new elements.

Also, there are some days to I try to get rid of this issue, and the first approach was to use personList.add() / personList.remove()
But no way to get the current index

Here below the code, with no error, but didn't work.
The navigation through the list didn't work


 
Christian Klugesherz
Greenhorn
Pie
Posts: 26
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Sorry,
Here is the code. The copy past didn't work :-(
PS: I didn't find the possibility to re-edit the old message in the forum.

Conclusion:
Buttons: New/ Delete/Save seems to react.
Buttons:Previous/Next are still not working

Thanks for your help.


 
Junilu Lacar
Bartender
Pie
Posts: 8790
81
Android Eclipse IDE IntelliJ IDE Java Linux Mac Scala Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Christian Klugesherz wrote:
Jus something that I didn't undrstood.
Notice also that on line 19, the declared type should be the interface type List, not the implementation type ArrayList.

What did I wrong ?

You wrote:

I suggested you change that to:

The difference being that the declared type, List<Person>, is an interface, not an implementation, which is what ArrayList<Person> is. What you did is not exactly wrong but it's poor form.
 
Junilu Lacar
Bartender
Pie
Posts: 8790
81
Android Eclipse IDE IntelliJ IDE Java Linux Mac Scala Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Christian Klugesherz wrote:
Here below the code, with no error, but didn't work.
The navigation through the list didn't work

No, it wouldn't work if you try it this way. You mixed and matched both mine and Campbell's comments. You have to choose one or the other, not mix both approaches. By using a local ListIterator, you clobber your current place in the list. The current place in the list needs to be remembered between calls to next() and previous() and any other method that would move the "cursor" to the current element.

Line 88 is where your problem lies.  You are calling the next() method again. That makes your cursor move again before calling the getName() method. That is, if your list had this:

... ["Johnny"] --> ["Erika"] --> ["Steve"]
and Line 87 resulted in the iterator pointing to "Johnny", then line 88 would again move the iterator and point to "Erika" and that's what the getName() method will return.  You need to assign the result of the call to next() on line 87 to a local variable, then call that object's getName method instead.

A similar issue exists for your "previous" logic.
 
Christian Klugesherz
Greenhorn
Pie
Posts: 26
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Again thanks for your prompt answers.

Be care, this is the wrong code you pointed out.
As explained above , there was an issue in the copy past. I updated a new answer in the forum with the corrected code.
Please see above.

OK, Following you approach, (If understood) I remodified the code, but also here it didn't work.
Each time when I click on “next” I get:
Next index (before next() ) =0
Next index (after next() ) =1
Next index (before next() ) =0
Next index (after next() ) =1


The iterator index didn't increase with : "personIterator.next();"

It's a nightmare.



 
Junilu Lacar
Bartender
Pie
Posts: 8790
81
Android Eclipse IDE IntelliJ IDE Java Linux Mac Scala Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Christian Klugesherz wrote:
OK, Following you approach, (If understood) I remodified the code, but also here it didn't work.
Each time when I click on “next” I get:
Next index (before next() ) =0
Next index (after next() ) =1
Next index (before next() ) =0
Next index (after next() ) =1


The iterator index didn't increase with : "personIterator.next();"

It's a nightmare.

Again, you have complicated your logic by trying to follow Campbell's advice to use a local ListIterator and your approach of using the ListIterator as a cursor to your list.  You CANNOT mix both approaches and still maintain simplicity and coherence in your code.

The local ListIterator will ALWAYS reset your place in the list to the beginning. This means that your navigation controls will not be relative to where the last operation left off; rather, they will be ALWAYS relative to the first element of the list.

I personally think that your approach of using a shared ListIterator was fine, as long as you stay consistent and use it all throughout the program.  If you are going to use a local ListIterator, then you'll have to deal with tracking your current position in the list yourself.  IMO, that just makes your program more complicated.
 
Christian Klugesherz
Greenhorn
Pie
Posts: 26
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Sorry for my instance.

There is something that I didn't catch.
Meaning that I have to forget the iterator, and to address the approach with a global variable, representing the position of the index.
Which method to use then, to move to the next element.

Thanks to the "List"'s method "iterator" like below ?


Merci

Christian
 
Piet Souris
Rancher
Posts: 1527
32
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
If 'currentIndex' is that 'global:variable', then it could simply be

(taking care that currentIndex is always a valid index)
 
Christian Klugesherz
Greenhorn
Pie
Posts: 26
1
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Finally I got it.
Very very simple with a global index variable
I remember that I decided to use iterator, was that in a former version, I wanted to use Combobox.
I didn't remember the exact reason any more.
In all the cases, it is now working as expected.
For that: Many Thanks to you for your help

The final code :-)

 
Piet Souris
Rancher
Posts: 1527
32
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Beware that personListID is always valid. For instance, when it is equal to list.size() - 1 and you delete the last record, what happens then? Did you test that situation?
 
Christian Klugesherz
Greenhorn
Pie
Posts: 26
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Sure Piet, there is a test which is missing . Thanks

 
Junilu Lacar
Bartender
Pie
Posts: 8790
81
Android Eclipse IDE IntelliJ IDE Java Linux Mac Scala Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Christian Klugesherz wrote:Sure Piet, there is a test which is missing . Thanks

Does this code allow you to delete the first Person on the list?
 
Christian Klugesherz
Greenhorn
Pie
Posts: 26
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
No of course.
This is what I wanted to do. :-)
Thanks



 
Junilu Lacar
Bartender
Pie
Posts: 8790
81
Android Eclipse IDE IntelliJ IDE Java Linux Mac Scala Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Interesting. If you purposely wrote the code so it's not possible to delete the first item in the list, you should probably add a comment that explains why, otherwise people like me will think you have a bug for a corner case.
 
Piet Souris
Rancher
Posts: 1527
32
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
OP should consider his strategy when it comes to adding or deleting records. For instance: what if there is only one record that gets deleted? A method that checks if the current index is valid seems handy (and what to do if that is not the case)
 
Christian Klugesherz
Greenhorn
Pie
Posts: 26
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Finally I completed it with the ComboBox.

Works as expected




 
Campbell Ritchie
Sheriff
Posts: 51368
87
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Well done
But don't use bounds for your display components; use a display manager instead.
 
Christian Klugesherz
Greenhorn
Pie
Posts: 26
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
OK for 
a display manager instead.


As I didn't find the method to modify the name of the combobox item, I decided to delete + recreate it
So it is mandatory to add the test I forgot  

 
Christian Klugesherz
Greenhorn
Pie
Posts: 26
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks to you all

Christian
 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic