Solution for swapping ArrayList data members with another

Question

Two possible solutions but which one? Or is there a Better?

I have a class MP3Track with members: Track number (int), track name (String), artist (String), in faourites (boolean) etc. My catalogue class contains two ArrayLists, one containing all tracks (vectorMain) and the other containing favourites (vectorFav) which are copies of the tracks in the main catalogue.

I want the user to have the option of swapping two track numbers (which are unique in both arrays).

Sample printout of the main catalogue below: enter image description here

Both samples get the desired result but:

• (Solution 1) deleted as didn't work in reverse.
• (Solution 2) Below Seems rather cumbersome!
• (Solution 3) Below Seems more elegant but very long winded!

getMainIndex() - returns the index position in the ArrayList for a given track number or -1.

Here is Solution 2 (overcame problem of solution 1 - but ugly)

Here is start of method:

public void swapTrack(){
 int t1 = -1, t2 = -1;
 System.out.println(face.getSwapTrackMenu()); 
 System.out.print("1) Please enter first track number to swap: ");
 t1 = scan.readInt(); 
 System.out.print("2) Please enter second track number to swap: ");
 t2 = scan.readInt();
 MP3Track mp31 = null;
 MP3Track mp32 = null;

Now solution 2

if((getMainIndex(t1)!=-1)&&(getMainIndex(t2)!=-1)){
 String s1 = null;
 String s2 = null;
 for(MP3Track track : vectorMain){
 if(track.getTrackNo() == t1){
 s1 = track.getTitle();
 }
 if(track.getTrackNo() == t2){
 s2 = track.getTitle();
 }
 }
 for(MP3Track track : vectorMain){
 if(track.getTitle().equals(s1)){
 track.setTrackNo(t2);
 }
 if(track.getTitle().equals(s2)){
 track.setTrackNo(t1);
 }
 }
 for(MP3Track track : vectorFav){
 if(track.getTitle().equals(s1)){
 track.setTrackNo(t2);
 }
 if(track.getTitle().equals(s2)){
 track.setTrackNo(t1);
 }
 }
}

Here is Solution 3

 //Loop thorugh main (ArrayList) looking for both track numbers
 for(MP3Track track : vectorMain){
 if(track.getTrackNo() == t1){
 //If track 1 found assign it to temp var
 mp31 = vectorMain.get(getMainIndex(t1));
 }
 if(track.getTrackNo() == t2){
 //If track 2 found assign it to temp var
 mp32 = vectorMain.get(getMainIndex(t2));
 }
 }
 //Check if we found track 1
 if(vectorMain.contains(mp31)){
 //Set temp var new track number
 mp31.setTrackNo(t2);
 }
 //Check if we found track 2
 if(vectorMain.contains(mp32)){
 //Set temp var new track number
 mp32.setTrackNo(t1);
 }
 mp31 = null; //Remove pointer reference
 mp32 = null; //Remove pointer reference
 //loop thorugh Fav (ArrayList) looking for track numbers
 for(MP3Track track : vectorFav){
 if(track.getTrackNo() == t1){
 //Assign it to temp var
 mp31 = vectorFav.get(getFavIndex(t1));
 }
 if(track.getTrackNo() == t2){
 //Assign it to temp var
 mp32 = vectorFav.get(getFavIndex(t2));
 }
 }
 //Check if we found it first
 if(vectorFav.contains(mp31)){
 mp31.setTrackNo(t2);
 }
 //Check if we found it first
 if(vectorFav.contains(mp32)){
 mp32.setTrackNo(t1);
 }

Is there something I'm missing here? I've hit a wall as to a better solution!!

I'm new to Java so any suggestions, improvements & comments appreciated.

Many thanks

• rolfl: What is the purposeof the 'track number'? Is it the track number of an album? It appears that it is not the album track number but something used for internal processes... what is it?
• TripVoltage: Assignment was for it to have a track number, artist name etc...in the program track numbers are unique.
• TripVoltage: Assignment was for it to have a track number, artist name etc...in the program track numbers are unique. Also user can search Main by: track number, track title, artist name & album name. User is also able to sort Main & Favs by: track number, track title, artist name, album name (natural ordering). Other functions include: adding & deleting tracks.
• abuzittin gillifirca: It appears trackNo is an identity field. It should not be changed. Why would your user want to swap some arbitrary number used to identify tracks?

Answer 1

In Solution 3, assuming you initialize mp31 and mp32 to null, you can replace this

//Check if we found track 1
if(vectorMain.contains(mp31)){
 //Set temp var new track number
 mp31.setTrackNo(t2);
}

With:

//Check if we found track 1
if(mp31 != null) {
 //Set temp var new track number
 mp31.setTrackNo(t2);
}

There is really no need to check if the arraylist contains the object since you used the get(...) method on the arraylist to get it in the first place. Therefore, you know that if the object has been initialized (is not null), the arraylist contains the object.

Also, I would change the method signature for getMainIndex to also take the list to search in as an argument to the method. In that case, you don't need to have two methods (with probably a lot of duplicated code) getMainIndex and getFavIndex can be the same method, getIndexInList(int trackNumber, List<MP3Track> list).

---

I am not sure how your getMainIndex method works at the moment, but I assume the code mp31 = vectorMain.get(getMainIndex(t1)); within the loop is the same thing as mp31 = track;. Therefore, you can replace this entire loop:

for(MP3Track track : vectorMain){
 if(track.getTrackNo() == t1){
 //If track 1 found assign it to temp var
 mp31 = vectorMain.get(getMainIndex(t1));
 }
 if(track.getTrackNo() == t2){
 //If track 2 found assign it to temp var
 mp32 = vectorMain.get(getMainIndex(t2));
 }
}

With either:

for(MP3Track track : vectorMain){
 if(track.getTrackNo() == t1){
 //If track 1 found assign it to temp var
 mp31 = track;
 }
 if(track.getTrackNo() == t2){
 //If track 2 found assign it to temp var
 mp32 = track;
 }
}

Or:

int index = getMainIndex(t1);
if (index != -1)
 mp31 = vectorMain.get(index);
index = getMainIndex(t2);
if (index != -1)
 mp32 = vectorMain.get(index);

---

As for Solution 2, this code here does not guarantee uniqueness of trackNo if two songs have the same title:

for(MP3Track track : vectorMain){
 if(track.getTrackNo() == t1){
 s1 = track.getTitle();
 }
 if(track.getTrackNo() == t2){
 s2 = track.getTitle();
 }
}
for(MP3Track track : vectorMain){
 if(track.getTitle().equals(s1)){
 track.setTrackNo(t2);
 }
 if(track.getTitle().equals(s2)){
 track.setTrackNo(t1);
 }
}

What you are essentially doing here is that you first scan through the list to get the title of the matching songs, and then you search for their title to change their track number. Why not change their track number within the first loop? You don't need all the three loops you have in Solution 2. Instead, search through vectorMain once and change the track numbers there, then search through vectorFav and change it there.

---

Some other comments:

I am not sure about how exactly you are using these two vectors and your MP3Track objects, or your trackNumber values. It seems to me as if whenever you swap track numbers, you do it in both of the lists. Therefore, perhaps you should use the same object references in both of the list. Don't create new MP3Track objects to place in vectorFav, you can re-use the same objects. That way, it will be enough to search only the vectorMain for the tracks, because it's the same object it will also update in vectorFav. As I said, I don't know exactly how you are using the trackNumber values so this might fit for you at the moment, but if it doesn't fit then you might want to change your design of the project.

As far as I have seen of the usage of getMainIndex and getFavIndex, you only use those methods to first get the index in the list, and then get the MP3Track object itself. Consider returning an MP3Track (the object itself) instead of an int (the index of the object in the list). Then you should also rename the method to reflect this change in behavior.

• TripVoltage: Hi Simon thanks for your comments. i know that the boolean value in MP3Track is enough information I need to produce the same result with only one list. My objective here with the my Uni assignment solution was to explore the API's to gain experience etc so I made it a little more difficult for myself!
• Simon Forsberg: @user103388 What many programmers actually are looking for when they write code is simplicity. It's good that you give yourself a challenge, but simple and elegant code is often better than complex and difficult code.
• TripVoltage: You got me thinking now if I should whip it out and re-write it! I have to present my solution giving my reason behind the decisions I've made etc so will I include the challenge to myself then as an added exercise.
• TripVoltage: getMainIndex & getfavIndex is used throught the program: You have a good point.