Jump to content

[Java] Help with ArrayList please? Would appreciate it since i cant seem to understand the problem


The Hero of Time

Recommended Posts

So, to give you a quick run through, i have created a class Fighter, which represents a fighter with a combat style

 

ENUM: COMBAT

public enum Combat
{
   MAGIC,
   MELEE,
   RANGED
}

 

CLASS: FIGHTER

public class Fighter
{
   private char initial;
   private Combat combatStyle;
   /**
   *
   * @param _initial The first initial of the fighter
   * @param _combatStyle The combat style the fighter mastered
   */
   public Fighter(char _initial, Combat _combatStyle)
   {
   initial = _initial;
   combatStyle = _combatStyle;
   }

   public char getInitial()
   {
   return initial;
   }
   
   public Combat getCombatStyle()
   {
   return combatStyle;
   }
}

usage:

Fighter combatant = new Fighter('E',Combat.RANGED);

 

Now we have a class barracks, the barracks is full of fighters.

The ArrayList inside barracks that represents the barracks is:

private ArrayList<Fighter> fighters = new ArrayList<>();

now the class Barracks has a few methods.

CLASS: BARRACKS

/**
* Creates a fighter and puts him in the barracks
* @param lr the first initial of the fighter
* @param type the combat style the fighter mastered
*/

public void createFighter(char lr, Combat type)
{
Fighter r = new Fighter(Character.toUpperCase(lr),type);
fighters.add(r);
}

/**
* Creates fighters and puts them into the barracks
* @param lr the first initial of the fighter
* @param type the combat style the fighter mastered
* @param amount the amount of fighters
* @[member='Return']
*/
public void createFighters(char lr, Combat type, int amount)
{
for(int i = 0; i < amount; i++)
createFighter(lr,type);
}

 

now if you have a basic understanding of the classes, the problem i have is with this method. The problem is that the fighters will not be removed from the barracks. They will all be called by a phrase, and the list with calledfighters will be correctly filled, but they just wont be removed from the barracks if there are 2 fighters with the same initial

/**
* Calls all fighters that have initials that are in the name. Example: name = war. All fighters with initials 'w','a', or 'r' will be called and removed from the barracks.
* @param phrase The phrase with initials 
* @param combatStyle The combat style the fighter is specialized in
*/
public ArrayList<Fighter> callFighters(String phrase, Combat combatStyle)
{
   //make the phrase fully uppercase
   phrase = phrase.toUpperCase();

   //Used to check if we have enough fighters in our army
   ArrayList<Fighter> tempBarracks = new ArrayList<>();

   //fighters to remove from the barracks after they have been called
   ArrayList<Fighter> toRemoveFightersFromBarracks = new ArrayList<>();

   //list of fighters that are called from the barracks
   ArrayList<Fighter> calledFighters = new ArrayList<>();

   //fill the temporary barracks with the contents real barracks, so that we dont have to modify the real 
   barracks if we call fighters and dont have enough fighters
   tempBarracks.addAll(fighters);

   //Fill the arraylist with chars of the parameter name //war will be 'W','A','R'
   ArrayList<Character> nameInChars = new ArrayList<>();
   for(char chr: phrase.toCharArray())
   nameInChars.add(chr);

   //Call all fighters
   int i = 0;
   int removedFighters = 0;
   for(Fighter fgt: tempBarracks)
   {
      if (nameInChars.contains(fgt.getInitial()) && fgt.getCombatStyle().equals(combatStyle) && i < nameInChars.size())
      {
         Fighter r = new Fighter(nameInChars.get(i), combatStyle);
         calledFighters.add(r);

         for (Fighter fghtr : tempBarracks)
         {
            if (fghtr.getInitial() == nameInChars.get(i) && fghtr.getCombatStyle().equals(combatStyle) &&    removedFighters  == 0)
            {
               toRemoveFightersFromBarracks.add(fghtr);
               removedFighters++;
            }
         }
         removedFighters = 0;
         i++;
      }
   }
   //toRemoveFightersFromBarracks will now be Correctly filled with all the fighters from the given phrase

   for(Fighter fgt: toRemoveFightersFromBarracks) //HERE LIES THE PROBLEM I'M HAVING.
   {
      if(tempBarracks.contains(fgt))
      {
          //Will also be FALSE if indexOfRm = -1 as described below.
      }
      int indexOfRm = tempBarracks.indexOf(fgt); //Will be -1  if we call 2 fighters with the same initials and    combat style


      tempBarracks.remove(fgt); //Wont remove object fgt from tempBarracks, probably has something to do with the  
      fact that the index of fgt in tempBarracks is -1
   }
So basically if there is a second fighter with the same initial, the object fgt from toRemoveFightersFromBarracks will not be removable from tempBarracks, the index of that fighter in tempbarracks is -1, and contains returns false, but it does contain a fighter with that exact initial and that exact combat style
 
I've made a .gif to show it



if(calledFighters.size() == phrase.length())
{
//Finally: remove the fighters from the barracks!
this.fighters.clear();
this.fighters.addAll(tempBarracks);
return calledFighters;
}
else
{
System.out.println("We dont have enough fighters for this phrase");
return null;
}
}

gif:

POSvurV.gif

 

 

 

EDIT: Images instead since this editor is annoying

callFighters

SzP4xXM.png

BPEoVMm.png

 

Edited by The Hero of Time
Link to comment
Share on other sites

So, to give you a quick run through, i have created a class Fighter, which represents a fighter with a combat style

 

ENUM: COMBAT


public enum Combat
{
MAGIC,
MELEE,
RANGED
}

 

CLASS: FIGHTER

public class Fighter
{
private char initial;
private Combat combatStyle;
/**
*
* @param _initial The first initial of the fighter
* @param _combatStyle The combat style the fighter mastered
*/
public Fighter(char _initial, Combat _combatStyle)
{
initial = _initial;
combatStyle = _combatStyle;
}

public char getInitial()
{
return initial;
}

public Combat getCombatStyle()
{
return combatStyle;
}
}

usage:

Fighter combatant = new Fighter('E',Combat.RANGED);

 

Now we have a class barracks, the barracks is full of fighters.

The ArrayList inside barracks that represents the barracks is:

private ArrayList<Fighter> fighters = new ArrayList<>();

now the class Barracks has a few methods.

CLASS: BARRACKS

/**
* Creates a fighter and puts him in the barracks
* @param lr the first initial of the fighter
* @param type the combat style the fighter mastered
*/

public void createFighter(char lr, Combat type)
{
Fighter r = new Fighter(Character.toUpperCase(lr),type);
fighters.add(r);
}

/**
* Creates fighters and puts them into the barracks
* @param lr the first initial of the fighter
* @param type the combat style the fighter mastered
* @param amount the amount of fighters
* @[member='Return']
*/
public void createFighters(char lr, Combat type, int amount)
{
for(int i = 0; i < amount; i++)
createFighter(lr,type);
}

 

now if you have a basic understanding of the classes, the problem i have is with this method. The problem is that the fighters will not be removed from the barracks. They will all be called by a phrase, and the list with calledfighters will be correctly filled, but they just wont be removed from the barracks if there are 2 fighters with the same initial

/**
* Calls all fighters that have initials that are in the name. Example: name = war. All fighters with initials 'w','a', or 'r' will be called and removed from the barracks.
* @param phrase The phrase with initials 
* @param combatStyle The combat style the fighter is specialized in
*/
public ArrayList<Fighter> callFighters(String phrase, Combat combatStyle)
{
//make the phrase fully uppercase
phrase = phrase.toUpperCase();

//Used to check if we have enough fighters in our army
ArrayList<Fighter> tempBarracks = new ArrayList<>();

//fighters to remove from the barracks after they have been called
ArrayList<Fighter> toRemoveFightersFromBarracks = new ArrayList<>();

//list of fighters that are called from the barracks
ArrayList<Fighter> calledFighters = new ArrayList<>();

//fill the temporary barracks with the contents real barracks, so that we dont have to modify the real barracks if we call fighters and dont have enough fighters
tempBarracks.addAll(fighters);

//Fill the arraylist with chars of the parameter name //war will be 'W','A','R'
ArrayList<Character> nameInChars = new ArrayList<>();
for(char chr: phrase.toCharArray())
nameInChars.add(chr);

//Call all fighters
int i = 0;
int removedFighters = 0;
for(Fighter fgt: tempBarracks)
{
if (nameInChars.contains(fgt.getInitial()) && fgt.getCombatStyle().equals(combatStyle) && i < nameInChars.size())
{
Fighter r = new Fighter(nameInChars.get(i), combatStyle);
calledFighters.add(r);

for (Fighter fghtr : tempBarracks)
{
if (fghtr.getInitial() == nameInChars.get(i) && fghtr.getCombatStyle().equals(combatStyle) && removedFighters == 0)
{
toRemoveFightersFromBarracks.add(fghtr);
removedFighters++;
}
}
removedFighters = 0;
i++;
}
}
//toRemoveFightersFromBarracks will now be Correctly filled with all the fighters from the given phrase

for(Fighter fgt: toRemoveFightersFromBarracks) //HERE LIES THE PROBLEM I'M HAVING.
{
if(tempBarracks.contains(fgt))
{
    //Will also be FALSE if indexOfRm = -1 as described below.
}
int indexOfRm = tempBarracks.indexOf(fgt); //Will be -1  if we call 2 fighters with the same initials and combat style


tempBarracks.remove(fgt); //Wont remove object fgt from tempBarracks, probably has something to do with the fact that the index of fgt in tempBarracks is -1
 
So basically if there is a second fighter with the same initial, the object fgt from toRemoveFightersFromBarracks will not be removable from tempBarracks, the index of that fighter in tempbarracks is -1, and contains returns false, but it does contain a fighter with that exact initial and that exact combat style
 
I've made a .gif to show it

}

if(calledFighters.size() == phrase.length())
{
//Finally: remove the fighters from the barracks!
this.fighters.clear();
this.fighters.addAll(tempBarracks);
return calledFighters;
}
else
{
System.out.println("We dont have enough fighters for this phrase");
return null;
}
}

gif:

POSvurV.gif

 

If I understand you correctly, you are trying to write a method that gets all the fighters that have any of the initials in a String, and the same Combat style, and you want to remove those from the barracks and return them as a new list?

 

If so, why not get rid of all those Lists and just use an iterator:

private List<Fighter> callFighters(String initial, Combat combat) {
    String upperCaseInitial = initial.toUpperCase();
    List<Fighter> calledFighters = new ArrayList<>();
    for (Iterator<Fighter> fighterIterator = barracks.iterator(); fighterIterator.hasNext();) {
        Fighter fighter = fighterIterator.next();
        if (upperCaseInitial.contains(Character.toString(fighter.getInitial())) && fighter.getCombatStyle() == combat) {
            calledFighters.add(fighter);
            fighterIterator.remove();
        }
    }
    return calledFighters;
}
Edited by Explv
Link to comment
Share on other sites

can u format

yeah i reformatted it, ipb editor is annoying as fk

 

If I understand you correctly, you are trying to write a method that gets all the fighters that have any of the initials in a String, and the same Combat style, and you want to remove those from the barracks and return them as a new list?

 

If so, why not get rid of all those Lists and just use an iterator:

private List<Fighter> callFighters(String initial, Combat combat) {
    String upperCaseInitial = initial.toUpperCase();
    List<Fighter> calledFighters = new ArrayList<>();
    for (Iterator<Fighter> fighterIterator = barracks.iterator(); fighterIterator.hasNext();) {
        Fighter fighter = fighterIterator.next();
        if (upperCaseInitial.contains(Character.toString(fighter.getInitial())) && fighter.getCombatStyle() == combat) {
            calledFighters.add(fighter);
            fighterIterator.remove();
        }
    }
    return calledFighters;
}

Because i'm trying to learn from this and understand what's causing this weird problem

Link to comment
Share on other sites

created an image instead, easier to read

 

nvm

sCTCw.png

 

This is the problem in my eyes, not only does it look like shit, but its where your mistake lies.

 

You are looping through every Fighter again for every fighter,, and then checking if removedFighter == 0

If it is == 0, then you are adding 1 to it, making the rest of the loop pointless, as its now equals 1, and it wont add any more fighters to the remove list.

 

 

 edited

 

sCTCw.png

 

 

 

just change to

 

for(Fighter fgt : tempBarracks){
    if(nameInChars.contains(fgt.getInitial()) && fgt.getCombatStyle.equals(combatStyle) && i < nameInChars.size()){
        calledFighters.add(new Fighter(nameInChars.get(i), combatStyle));
        if(fgt.getInitial() == nameInChars.get(i)){
            toRemoveFightersFromBarracks.add(fgt);
        }
    }
    i++;
}
Edited by Tom
  • Like 1
Link to comment
Share on other sites

 

nvm

sCTCw.png

 

This is the problem in my eyes, not only does it look like shit, but its where your mistake lies.

 

You are looping through every Fighter again for every fighter,, and then checking if removedFighter == 0

If it is == 0, then you are adding 1 to it, making the rest of the loop pointless, as its now equals 1, and it wont add any more fighters to the remove list.

 

 

 edited

 

just change to

for(Fighter fgt : tempBarracks){
    if(nameInChars.contains(fgt.getInitial()) && fgt.getCombatStyle.equals(combatStyle) && i < nameInChars.size()){
        calledFighters.add(new Fighter(nameInChars.get(i), combatStyle));
        if(fgt.getInitial() == nameInChars.get(i)){
            toRemoveFightersFromBarracks.add(fgt);
        }
    }
    i++;
}

No, that's not the problem. the list will be filled correctly. The reason why it looks like shit is because i've tried cleaner methods with less code, but none of them work. in every scenario the list will be filled correctly, but when i try to remove fighters from the barracks from the contents of toRemoveFightersFromBarracks, it will never work with duplicates

 

the reason why i posted this topic is to understand what the problem is exactly.

 

also with your code the toremove list contains 2 'W' 's and mine contains warw which is correct 

(  barracks.callFighters("warw", Combat.RANGED); )

Edited by The Hero of Time
Link to comment
Share on other sites

No, that's not the problem. the list will be filled correctly. The reason why it looks like shit is because i've tried cleaner methods with less code, but none of them work. in every scenario the list will be filled correctly, but when i try to remove fighters from the barracks from the contents of toRemoveFightersFromBarracks, it will never work with duplicates

 

the reason why i posted this topic is to understand what the problem is exactly.

 

also with your code the toremove list contains 2 'W' 's and mine contains warw which is correct 

(  barracks.callFighters("warw", Combat.RANGED); )

Have you ever thought about using a hashmap?

Link to comment
Share on other sites

The problem you are facing is you are comparing fighters by their initials and combat level. If you add two "a" fighters with same combat, when you go through your loop of fighters in the second for loop (the one nested in the first), you're comparing the initial and Combat enums. This means, it will always pick the first Fighter in the Barracks with those initials/Combat enum, and will think it needs to remove that specific Fighter from the Barracks twice. This results in the -1 you are seeing, as the second time around the Fighter is already gone.
 
If you wanna stick with your current solution, simply replace:

if (fghtr.getInitial() == nameInChars.get(i) && fghtr.getCombatStyle().equals(combatStyle) && removedFighters  == 0)

with

if (fghtr == fgt)

This will result in the correct fighter being removed. This is because we are checking if its the same fighter object, not that it just has the same initials/Combat enum. Now, you are reiterating through lists for no real reason, and I think your logic is slightly off (calling "aabb" if the array has 4 "a"s would return 4 "a"s instead of 2 a and 2 b), here is a simpler solution:

 
http://pastebin.com/LmngC84M
 
And from there, you could do:

barracks.createFighters('a', Combat.MELEE, 3);
barracks.createFighters('b', Combat.MELEE, 3);
barracks.createFighters('c', Combat.MELEE, 3);
barracks.callFighters("aabbcc", Combat.MELEE);

which results in:

We have 9 fighters.
Removing A:MELEE from barracks
Removing A:MELEE from barracks
Removing B:MELEE from barracks
Removing B:MELEE from barracks
Removing C:MELEE from barracks
Removing C:MELEE from barracks
We removed 6 fighters, leaving us with 3 in the barracks.
  • Like 1
Link to comment
Share on other sites

 

The problem you are facing is you are comparing fighters by their initials and combat level. If you add two "a" fighters with same combat, when you go through your loop of fighters in the second for loop (the one nested in the first), you're comparing the initial and Combat enums. This means, it will always pick the first Fighter in the Barracks with those initials/Combat enum, and will think it needs to remove that specific Fighter from the Barracks twice. This results in the -1 you are seeing, as the second time around the Fighter is already gone.

 

If you wanna stick with your current solution, simply replace:

if (fghtr.getInitial() == nameInChars.get(i) && fghtr.getCombatStyle().equals(combatStyle) && removedFighters  == 0)

with

if (fghtr == fgt)

This will result in the correct fighter being removed. This is because we are checking if its the same fighter object, not that it just has the same initials/Combat enum. Now, you are reiterating through lists for no real reason, and I think your logic is slightly off (calling "aabb" if the array has 4 "a"s would return 4 "a"s instead of 2 a and 2 b), here is a simpler solution:

 

http://pastebin.com/LmngC84M

 

And from there, you could do:

barracks.createFighters('a', Combat.MELEE, 3);
barracks.createFighters('b', Combat.MELEE, 3);
barracks.createFighters('c', Combat.MELEE, 3);
barracks.callFighters("aabbcc", Combat.MELEE);

which results in:

We have 9 fighters.
Removing A:MELEE from barracks
Removing A:MELEE from barracks
Removing B:MELEE from barracks
Removing B:MELEE from barracks
Removing C:MELEE from barracks
Removing C:MELEE from barracks
We removed 6 fighters, leaving us with 3 in the barracks.

Thank you, this is exactly what i was asking. i dont care about the solution i just want to know/understand what was wrong. cheers man biggrin.png

 

Edited by The Hero of Time
Link to comment
Share on other sites

  • 2 months later...
for(Fighter fgt : tempBarracks){
    if(nameInChars.contains(fgt.getInitial()) && fgt.getCombatStyle.equals(combatStyle) && i < nameInChars.size()){
        calledFighters.add(new Fighter(nameInChars.get(i), combatStyle));
        if(fgt.getInitial() == nameInChars.get(i)){
            toRemoveFightersFromBarracks.add(fgt);
        }
    }
    i++;
}

Instead of i, you can use

tmpBarracks.indexOf(fgt)

, or if u just want to use variable i, use normal for (int i =0 ...) 
i++ in foreach looks bad 

or try stream

tmpBarracks.stream().filter(p->{

if(nameInChars.contains(p.getInitial()) && p.getCombatStyle.equals(combatStyle) && tmpBarracks.indexOf(p) < nameIncars.size()){

	calledFighters.add(new fighter(nameInchars.get(tmpBarracks.indexOf(p), combatStyle));

		if(p.getInitial().equals(nameInChars.get(tmpBarracks.indexOf(p)){

			return true;

		}

}else {return false;})
.collect(Collectors.toList())

Streams are pretty useful, code looks better. 

BUT REMEMBER BECAREFUL WITH list.indexOf() !!! 

if u have 2 the same objects in list, its going to get first element all the time! 

 

also this if   

  if(nameInChars.contains(fgt.getInitial()) && fgt.getCombatStyle.equals(combatStyle) && i < nameInChars.size()){

looks too long, move this things to other method. 

Edited by Efpkaf
Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

  • Recently Browsing   0 members

    • No registered users viewing this page.
×
×
  • Create New...