Checking if strings exist inside an ArrayList list and adding a goal point a to a player string?

[Justin Robbins]

I have a class called GameCollector which adds up all the points between rival teams. I also made a Game class that has a constructor that takes in two separate strings to represent teamOne and teamTwo. And also a Team class with a constructor that has four parameters, string teamName, int totalGoals, int games, and int wins. And a player class that has a constructor with parameters String name, int goal, String team. Inside the GameCollector class I have this method:

public boolean goalScored(String first_Team, String second_Team, String wat_Team, String thePlayer) {
    for (Game g : games) {
        if(!(g.getFirstTeamName().equals(first_Team) && g.getSecondTeamName().equals(second_Team))) {
            return false;
        }
        if(!(g.getFirstTeamName().equals(wat_Team) || g.getSecondTeamName().equals(wat_Team))) {
            return false;
        }
    }

    Player p = new Player(thePlayer, 1, wat_Team);
    return true;
}

Not sure if this method is implemented right but I want this method to be called to show that thePlayer has scored a goal for wat_Team, that is playing between the first_Team and the second_Team. With a goal being +1. A false boolean should be figured out through if first_Team and second_Team are not playing against each other. Of course, if both firstTeam and secondTeam are currently playing a game but whatTeam isn't the name of either, it should return a false boolean. But if those check out then a score should be added to thePlayer that the whatTeam refers in the game between firstTeam and secondTeam.

Thank you

[Carey Brown]

With your loop you go through each game, however, when you hit the first game where one of your if()'s are true, you stop processing any remaining games and return false.
I'm guessing that that is not what you intended.

[Carey Brown]

On line 11 you create a new Player 'p', but then throw it away without doing anything with it.

[Justin Robbins]

Carey Brown wrote:On line 11 you create a new Player 'p', but then throw it away without doing anything with it.

with the loops I want to check if there is a game in which the first and second are playing each other, if not then false.
If there is a game between them I want to check if either of those team names are "whatTeam", if that is the case I then
want to add a goal of 1 to the player player in the parameter, to which the fourth parameter refers to in between the two
teams referred to in the first two parameters.

[Paul Clapham]

Carey has pointed out some of the problems with your implementation of that method, but I have to say that I don't understand what the method is supposed to be doing in the first place. Even after reading your description several times I'm still lost.

I don't understand what it means for a team to be playing between two other teams, for one thing: you said

I want this method to be called to show that thePlayer has scored a goal for wat_Team, that is playing between the first_Team and the second_Team

But probably I've misinterpreted what you meant to say there. So let's take an example. Suppose first_Team is "Manchester United" and second_Team is "Toronto Maple Leafs" and wat_Team is "Portland Trailblazers". Can you explain what we should be looking for to see if that's a valid combination, or whatever? Remember I don't know what a Game is in your data model, sometimes you describe it as if it's supposed to be taking place right now but other times it seems like it could represent a game which has already taken place.

[Junilu Lacar]

1. Don't get cute with names. I'm not sure what you're trying to convey with a name like wat_Team but it gets old after reading it a couple of times.

2. The design of this method smells to high heaven.  Java is an object-oriented language. With as many different objects that you have involved here, this is not at all an object-oriented way to accomplish anything.

3. This poorly designed method is probably not the only thing that smells. Poorly designed methods usually arise from some need that must be satisfied in the context of poorly designed code. Because the surrounding code is poorly design and responsibilities are not properly assigned to appropriate objects, the need can only be satisfied by designing yet another badly formed method call that violates even more design principles than what the code around it does. This is a smell of a vicious cycle in your code: Crap code begets more crap code begets more crap code. This will lead you to places you wouldn't wish on your worst enemy.

The only way to get out of a vicious cycle is to start cleaning up your code and making better design choices. You want to get into a virtuous cycle instead where Good code begets more good code begets more good code.

[Justin Robbins]

Junilu Lacar wrote:1. Don't get cute with names. I'm not sure what you're trying to convey with a name like wat_Team but it gets old after reading it a couple of times.

2. The design of this method smells to high heaven.  Java is an object-oriented language. With as many different objects that you have involved here, this is not at all an object-oriented way to accomplish anything.

I know I need help badly. How can I clean this up and stay true OO design principles?

[Junilu Lacar]

Justin Robbins wrote:I know I need help badly. How can I clean this up and stay true OO design principles?

First, let's figure out which design principles you know.  A couple of foundational ones are separation of concerns and assignment of responsibilities to the proper objects.  If you're not sure you know what these are about, then you don't know what they're about. Go read up on it first.  Another concept is divide and conquer or functional decomposition. Again, if you're not sure what these are, go and read up on them. You don't need to read these in depth. Just get a general idea of the principle, maybe look at some examples, then come back.

You're going to have to start over, most likely. Sometimes that's the quickest way to clean things up, if you don't have 10,000 lines of code already written.

[Carey Brown]

I think some use cases would help get you in the right direction. Example (you'd need to tell us if these are in fact correct):

A Team consists of a number of Players

There is one-or-more Games G where Team A plays Team B

Player P could play for different Teams for different Games

When Player P scores his/her score count needs to be incremented

more?

[Justin Robbins]

Carey Brown wrote:I think some use cases would help get you in the right direction. Example (you'd need to tell us if these are in fact correct):

A Team consists of a number of Players

There is one-or-more Games G where Team A plays Team B

Player P could play for different Teams for different Games

When Player P scores his/her score count needs to be incremented

more?

Yes, the team class has players.
There are more or more games where Team A plays team B.
Player P can play with different teams for different games.
Player P's score count needs to be incremented.

Yes to all. I just don't know how to achieve that. My OO is weak.

[Justin Robbins]

Paul Clapham wrote:Carey has pointed out some of the problems with your implementation of that method, but I have to say that I don't understand what the method is supposed to be doing in the first place. Even after reading your description several times I'm still lost.

I don't understand what it means for a team to be playing between two other teams, for one thing: you said

I want this method to be called to show that thePlayer has scored a goal for wat_Team, that is playing between the first_Team and the second_Team

But probably I've misinterpreted what you meant to say there. So let's take an example. Suppose first_Team is "Manchester United" and second_Team is "Toronto Maple Leafs" and wat_Team is "Portland Trailblazers". Can you explain what we should be looking for to see if that's a valid combination, or whatever? Remember I don't know what a Game is in your data model, sometimes you describe it as if it's supposed to be taking place right now but other times it seems like it could represent a game which has already taken place.

Player can play for either team. And whatTeam refers to the team that player is on. It was confusing wording sorry.

[Carey Brown]

So if you know the Player who scored what difference does it make which Game or which Team?

[Justin Robbins]

Carey Brown wrote:So if you know the Player who scored what difference does it make which Game or which Team?

I think the points gets calculated differently for each individual team the player plays for.

[Junilu Lacar]

If the Game has rules like football (or soccer, if you're in a country that calls it that   ), then a Player can score an "own goal" and the points for the goal actually go to the other team. Do you need to account for a situation like this?

[Junilu Lacar]

Justin Robbins wrote:I think the points gets calculated differently for each individual team the player plays for.

Ok, I think it's time you give us the entire context of the problem. We can be here all week if we have to keep guessing what all the details of this problem are.

[Carey Brown]

Justin Robbins wrote:

Carey Brown wrote:So if you know the Player who scored what difference does it make which Game or which Team?

I think the points gets calculated differently for each individual team the player plays for.

Yep, seems like some research into the requirements are in order before continuing any further.

[Paul Clapham]

Justin Robbins wrote:Player can play for either team. And whatTeam refers to the team that player is on. It was confusing wording sorry.

So one might ask "How many goals did Shaquille O'Neal score while playing for Manchester United against Montreal Canadiens?" for example? I always find it easier to work with actual examples, but in the end a small number of examples aren't sufficient and you need accurately-worded specs.