trjhtr

I have many lines of code that - to me - look very odd, and I would like to condense, but I don't exactly know how.

This is the part I dislike the most:

if(random.nextInt(50) == 26) 
 int rnteam = random.nextInt(twoteams.size());
 String rndteam = "";
 if(rnteam == 0) rndteam = team1.getName();
 else if(rnteam == 1) rndteam = team2.getName();
 String rndplayfrndteam = "";
 int rndplayer = random.nextInt(5);
 if(rndteam == team1.getName()) rndplayfrndteam = team1.getPlayers().get(rndplayer);
 else if(rndteam == team2.getName()) rndplayfrndteam = team2.getPlayers().get(rndplayer);
 // Determining this player's score
 // Team 1
 if(rndplayfrndteam == team1.getPlayers().get(0)) t1p1score += scoreAmount;
 if(rndplayfrndteam == team1.getPlayers().get(1)) t1p2score += scoreAmount;
 if(rndplayfrndteam == team1.getPlayers().get(2)) t1p3score += scoreAmount;
 if(rndplayfrndteam == team1.getPlayers().get(3)) t1p4score += scoreAmount;
 if(rndplayfrndteam == team1.getPlayers().get(4)) t1p5score += scoreAmount;
 // Team 2
 if(rndplayfrndteam == team2.getPlayers().get(0)) t2p1score += scoreAmount;
 if(rndplayfrndteam == team2.getPlayers().get(1)) t2p2score += scoreAmount;
 if(rndplayfrndteam == team2.getPlayers().get(2)) t2p3score += scoreAmount;
 if(rndplayfrndteam == team2.getPlayers().get(3)) t2p4score += scoreAmount;
 if(rndplayfrndteam == team2.getPlayers().get(4)) t2p5score += scoreAmount;

 if(rnteam == 0) 
 t1sofar += scoreAmount;
 else 
 t2sofar += scoreAmount;
 
 int t1scores = t1p1score, t1p2score, t1p3score, t1p4score, t1p5score;
 int t1maxScore = findGreatestInt(t1scores);
 int t1bestPlayer = 0;
 if(t1maxScore == t1p1score) t1bestPlayer = 1;
 if(t1maxScore == t1p2score) t1bestPlayer = 2;
 if(t1maxScore == t1p3score) t1bestPlayer = 3;
 if(t1maxScore == t1p4score) t1bestPlayer = 4;
 if(t1maxScore == t1p5score) t1bestPlayer = 5;

 int t2scores = t2p1score, t2p2score, t2p3score, t2p4score, t2p5score;
 int t2maxScore = findGreatestInt(t2scores);
 int t2bestPlayer = 0;
 if(t2maxScore == t2p1score) t2bestPlayer = 1;
 if(t2maxScore == t2p2score) t2bestPlayer = 2;
 if(t2maxScore == t2p3score) t2bestPlayer = 3;
 if(t2maxScore == t2p4score) t2bestPlayer = 4;
 if(t2maxScore == t2p5score) t2bestPlayer = 5;
 hangOver(rndplayfrndteam + " scored " + scoreAmount, team1, team2, t1bestPlayer, t1maxScore, t2bestPlayer, t2maxScore, t1sofar, t2sofar);
 else 
 int t1scores = t1p1score, t1p2score, t1p3score, t1p4score, t1p5score;
 int t1maxScore = findGreatestInt(t1scores);
 int t1bestPlayer = 0;
 if(t1maxScore == t1p1score) t1bestPlayer = 1;
 if(t1maxScore == t1p2score) t1bestPlayer = 2;
 if(t1maxScore == t1p3score) t1bestPlayer = 3;
 if(t1maxScore == t1p4score) t1bestPlayer = 4;
 if(t1maxScore == t1p5score) t1bestPlayer = 5;

 int t2scores = t2p1score, t2p2score, t2p3score, t2p4score, t2p5score;
 int t2maxScore = findGreatestInt(t2scores);
 int t2bestPlayer = 0;
 if(t2maxScore == t2p1score) t2bestPlayer = 1;
 if(t2maxScore == t2p2score) t2bestPlayer = 2;
 if(t2maxScore == t2p3score) t2bestPlayer = 3;
 if(t2maxScore == t2p4score) t2bestPlayer = 4;
 if(t2maxScore == t2p5score) t2bestPlayer = 5;
 hangOver("No one", team1, team2, t1bestPlayer, t1maxScore, t2bestPlayer, t2maxScore, t1sofar, t2sofar);

All of the if-statements combined just look bad, and aren't very efficient. Is there a way I could change these into a loop and it still work? If so, how?

Here are my comments in order of severity:

1) BUGS

1. Test equality of Strings using equals() not ==



2. 
   

   use of else if
   
   let's examine this piece of code

   
   
   

   String rndteam = "";
if(rnteam == 0) rndteam = team1.getName();
else if(rnteam == 1) rndteam = team2.getName();

   
   

it assigns a value to the String rndteam. now, I assume that the logic was meant that rndteam should be assigned the name of either team1 or team2 but the code actually allows for both if statements to be false and the value will then be an empty String. You can fix that by changing the flow to an if else one

String rndteam = ""; 
if(rnteam == 0) rndteam = team1.getName(); 
else rndteam = team2.getName(); 

in my eyes, in the case of deciding between two options, the short form of the if statement can be used to make it clear:

String rndteam = rnteam == 0 ? team1.getName() : team2.getName(); 

2) Design problems

1. 
   

   Now we come to the question of all those if statements. You already have arrays that you assign the variables into so why not use them when you ask about the team players?

   
   
   

   int t1scores = new int[team1.size()];
for (int i = 0 ; i < team1.size(); i++)
if(rndplayfrndteam.equals(team1.getPlayers().get(i))) t1scores[i] += scoreAmount;
   

   
   



2. But really, as it was already suggested, you should design a class that holds all the information of a Player and another for team.

Once you have a number of variables with similar names, such as t?p?score, you should consider changing these to arrays (a 2D one would make sense in your case), which you can then iterate over in a loop to reduce the repetition.

What is so special about the if(random.nextInt(50) == 26) condition? Some constants would be useful here.

Try to avoid removing letters from variable names just to shorten them, lest they lose their meaning. I can't work out what rndplayfrndteam means.

A method called hangOver tells me absolutely nothing about what it does, and you should consider a different name.

You may wish to consider some data classes to keep the players and their scores together, possibly including their teams as well, to keep things a bit cleaner.