0
private List<Actor> actors = new ArrayList<>();

public void addActors(Movie x)
{
if(actors.isEmpty())
{
    actors.addAll(x.getListOfActors());
}
else
{
    List<Actor> movieActors = x.getListOfActors();
    ArrayList<Actor> tempActors= new ArrayList<Actor>();
    for(int z = 0 ; z <actors.size(); z++)
    {

        for(int n = 0 ; n <movieActors.size();n++)
        {
            if(actors.get(z).getName().equals(movieActors.get(n).getName()))
                actors.get(z).addCount(); // actor count 
            else
            {
                tempActors.add(movieActors.get(n));
            }
        }
    }
    actors.addAll(tempActors);
}
}

So each movie object have a list of actors. And each actor objects have a int count object which is initialized at 1. Everything I run addActors(Movie x), i want to check if there is any actors in movie x that is in List actors. if there is, i want to increment count by 1 else just add it to List actors. This is my current way of doing it, i realized its wrong but i dont know how to fix it. I run the method twice or more, repeated actors will be added to List actors because of actors.addAll(tempActors). Is there a better way to do this ?

update: So the problem isnt with the string comparing. Even if i use .equals, it isnt the result i want. sample test case:

movie: "frozen"

actors: Kristen Bell, Idina Menzel, Jonathan Groff, Josh Gad

movie: "the boss"

actors: Melissa McCarthy, Kristen Bell, Peter Dinklage, Ella Anderson

so if I use addActors(movie x) on these two movies, each actors should only appear once. In the List actors. And count for kristen bell should be at 2 now. everyone else is 1.(count is number of movies they are in)

however when i run this method with these two movies, my List actors now contains:

Kristen Bell Idina Menzel Jonathan Groff Josh Gad Melissa McCarthy Peter Dinklage Ella Anderson Melissa McCarthy Kristen Bell Peter Dinklage Ella Anderson Melissa McCarthy Kristen Bell Peter Dinklage Ella Anderson Melissa McCarthy Kristen Bell Peter Dinklage Ella Anderson

(sorry for long explanation, I don't know how else to explain this )

  • 1
    Possible duplicate of [How do I compare strings in Java?](http://stackoverflow.com/questions/513832/how-do-i-compare-strings-in-java) – denvercoder9 Dec 05 '16 at 06:08
  • No, the problem is not comparing strings. using == or .equals gives me same result. – Jackie Chen Dec 05 '16 at 06:13
  • @JackieChen by incrementing actor count what do you want to achieve? Do you want to save this count somewhere or what? – Naman Dec 05 '16 at 06:14
  • count in each actor is the number of movies they are in , so i add two movies with same actors, the actor count should now be 2. @nullpointer – Jackie Chen Dec 05 '16 at 06:21
  • you can maintain a `Map` (not literally | more of `Map`) in general to store the count. and inside your `if` increment the count for each actor key. – Naman Dec 05 '16 at 06:25
  • okay , but how would i added other actors that are not in the List actors to the list ? @nullpointer – Jackie Chen Dec 05 '16 at 06:30
  • @JackieChen that is already taken care by your `else` code I guess. You are adding the actors not present already in a list to `tempActors` and eventually to `actors` – Naman Dec 05 '16 at 06:32
  • @nullpointer I think my problem is i'm repeatedly adding the same actors in each loop because of that else statement. – Jackie Chen Dec 05 '16 at 06:35
  • oh if you don't want duplicates , you can use `Set` instead of `List` – Naman Dec 05 '16 at 06:36
  • You store the count of occurence in `Actor` for each one ? I don't think an actor should store this information since this depends on the list you are comparing. – AxelH Dec 05 '16 at 06:42
  • I was think if i can do this in one loop , it should be fine. But set would work so thanks! @nullpointer – Jackie Chen Dec 05 '16 at 06:43

6 Answers6

1

Few open points --

One, I would suggest using foreach for DS like List to iterate through. Take a look at How does the Java 'for each' loop work?

Two, use equals to compare strings. Help - How do I compare strings in Java?

Third, use a counter variable, e.g. int counter = 0; which can be used to count the number of duplicate occurrences as

if(actors.get(z).getName().equals(movieActors.get(n).getName())) {
    counter++; //currently just saving the count, you can save the name as well
}

Updating from the comments discussion, to keep a count of actors that appeared in different movies :

You can maintain a Map<Actor,Count> (not literally | more of Map<String, Integer>) in general to store the count. and inside your if increment the count for each actor key.

Further to keep a unique list of actors, would suggest using Set instead of ArrayList. Please read How to use java.Set

Community
  • 1
  • 1
Naman
  • 27,789
  • 26
  • 218
  • 353
  • Im not sure how a counter would help? Maybe I didnt explain it well but i edit it just now . The actor count variable is the number of movies they are in so if i added 2 movies with same actor in both movie, the count of that actor would be 2 now because that actor is in the two movies. @nullpointer – Jackie Chen Dec 05 '16 at 06:28
1

Remove every thing from else condition and replace with following code. Though its not an optimal solution but might serve your purpose.

else {
        List<Actor> movieActors = (List<Actor>) x.getListOfActors();
        for (int z = 0; z < x.getListOfActors().size(); z++) {
            Actor actor = movieActors.get(z);
            if (actors.contains(actor)) {
                actors.get(actors.indexOf(actor)).addCount();
            }else{
                actors.add(actor);
            }
        }
    }

And you have to Override equals method in Actor class

@Override
public boolean equals(Object obj) {
    if(obj instanceof Actor){
        Actor a = (Actor)obj;
        if(this.name.equals(a.name)){
            return true;
        }
    }
    return false;

}
Ahmed Raaj
  • 508
  • 4
  • 16
0

When you do

actors.get(z).getName() == movieActors.get(n).getName()

I suppose name is a string, in that case it will not be true for same names. You need to do

actors.get(z).getName().equals(movieActors.get(n).getName())

Refer this - How do I compare strings in Java?

Community
  • 1
  • 1
xitter
  • 708
  • 5
  • 15
  • ok, ill fix that. But i think the initial problem will still stand because i'm looking through movieActors, z times which will keep adding duplicate actor objects. @vijay jain – Jackie Chen Dec 05 '16 at 06:09
  • Correct me, if I am wrong, It will add duplicates if there are already duplicates in actors or movieActors. In that case you can opt for a set/hashset and override equals and hashcode methods in Actors class. – xitter Dec 05 '16 at 06:13
  • you're correct , it keeps adding duplicates because its loop n*z times. Using hashset would work. If i can do this using one loop , it should be fine , but not sure how. – Jackie Chen Dec 05 '16 at 06:56
0

Use the ".equals" instead of "=="

0

Same as @Ahmed Raaj.

else{

        List<Actor> movieActors = x.getListOfActors();      
        List<Actor> duplicateActors = new ArrayList<>();
        List<Actor> uniqueActors = new ArrayList<>();

        for(Actor eachActor : movieActor){
            if(actors.contains(eachActor))
                duplicateActors.add(eachActor);
            else
                uniqueActors.add(eachActor);
        }

        for(Actor eachDuplicateActor : duplicateActors){
            for(Actor eachOriginalActors : actors){
                if(eachOriginalActors.getName().equals(eachDuplicateActor.getName())
                        eachOriginalActors.addCount();
            }
        }

        actors.addAll(uniqueActors);
    }

I haven't tested the code, so there might be errors

Jitin Kodian
  • 471
  • 4
  • 14
0

I think you should reverse the order of the loops, and only add the actor if it is not found:

for(int n = 0 ; n <movieActors.size();n++)
{
    boolean found = false;
    for(int z = 0 ; z <actors.size(); z++)
    {
        if(actors.get(z).getName().equals(movieActors.get(n).getName())) {
            actors.get(z).addCount(); // actor count
            found = true;
            break;
        }
    }
    if (!found) {
        tempActors.add(movieActors.get(n));
    }
}

But I don't get the signification of count

Maurice Perry
  • 9,261
  • 2
  • 12
  • 24