-3

I would like to make sure that the same film is added, but unfortunately, something is not working out. What am I doing wrong?

class Movie:
    def __init__(self, name, pegi, year):
        self.name = name
        self.pegi = pegi
        self.year = year

class Movies(Movie):

    def __init__(self):
        self.collection = []

    def addMovieToCollection(self, name, pegi, year):
        super().__init__(name, pegi, year)
        structure_of_movie = name + " - " + pegi + " - " + str(year)
        if structure_of_movie in self.collection :
            print("That movie like" + self.name + " already exists")
        else:
            self.collection.append(structure_of_movie)

    def showMovie(self):
        return print(*self.collection, sep='\n')

f = Movies()

f.addMovieToCollection("Iron Man 1", "blue sign", 1995)
f.addMovieToCollection("Iron Man 1", "blue sign", 1995)
f.addMovieToCollection("Iron Man 2", "+7", 1995)
f.addMovieToCollection("Iron Man 3", "+16", 1995)
f.addMovieToCollection("Iron Man 4", "+12", 1995)
f.addMovieToCollection("Iron Man 5", "+18", 1995)
f.showMovie()
   

Please Help me

powezka43
  • 45
  • 5
  • 6
    "something is not working" what is not working? How someone can understand what do you mean by "something" you should tell exactly what is not working – deadshot Nov 28 '20 at 08:49
  • the security of adding the same film does not work - foreach loop – powezka43 Nov 28 '20 at 08:51
  • 2
    You don't want to let the same film be added twice correct ? and the uniqueness check is only on the film name? not in the film name + year ? – Vikash Singh Nov 28 '20 at 08:52
  • this will help [Compare object instances for equality by their attributes](https://stackoverflow.com/questions/1227121/compare-object-instances-for-equality-by-their-attributes) – deadshot Nov 28 '20 at 08:52
  • @VikashSingh I would like to get it, but for now it does not work with the name itself – powezka43 Nov 28 '20 at 08:54
  • 1
    I mean, `self.collection.__eq__(CurrentFilm)` is going to compare the _whole_ `collection` to each of the films in it. It's like comparing a basket of apples to one single apple - they will _never_ be equal – ForceBru Nov 28 '20 at 08:54
  • As an aside, `super().__init__(name, pegi, year)` makes no sense in `addMovieToCollection`. And having `Movies` inherit from `Movie` also makes no sense. – juanpa.arrivillaga Nov 28 '20 at 08:59
  • @juanpa.arrivillaga so how do you inherit the constructor from the `Movie` class? – powezka43 Nov 28 '20 at 09:05
  • @powezka43 *in the constructor usually*. Why would you call the constructor every time you use `addMovieToCollection`? But again, *fundamentally* having `Movies` inherit from `Movie` is a broken design. A thing is not a collection of things. A collection of things *has things*. A collection of things is a separate type. Just *don't inherit* from `Movie` at all. Use *composition*, not inheritance. – juanpa.arrivillaga Nov 28 '20 at 09:06
  • @powezka43 Didn't you just update the question with working code from one of the answers? Why would you do that? Now the question "what am I doing wrong" makes 0 sense because the code works as expected and there is nothing _wrong_ with it. Could you please revert to the original code, clarify your question (provide expected and actual output) and mark the answer as accepted if it fixed your problem? – Czaporka Nov 28 '20 at 09:24

2 Answers2

2

Add __eq__ and __hash__ method in class, something like this:

class Movie:
    def __init__(self, name, pegi, year):
        self.name = name
        self.pegi = pegi
        self.year = year

    def __eq__(self, other): 
        if not isinstance(other, Movie):
            # don't attempt to compare against unrelated types
            return NotImplemented

        return self.name == other.name # and self.year == other.year
    
    def __hash__(self):
        return hash(self.name)
        # if you want to compare with year too.
        # return hash(self.name + str(self.year))   
      
    def __str__(self):
        # This is how each movie will be printed. 
        # You can improve the formatting here.
        return "Movie name: {}, Year: {}".format(self.name, self.year)

class Movies(): # it makes no sense that Movies should inherit Movie, don't do that.

    def __init__(self):
        self.collection = set()

    def addMovieToCollection(self, name, pegi, year):
        current_movie = Movie(name, pegi, year)
        if current_movie in self.collection:
            print("That movie like" + current_movie.name + " already exists")
        else:
            self.collection.add(current_movie)
            print("Added " + current_movie.name + " " + str(current_movie.year))
            
    def __str__(self):
        # Join the collection together how you would like to represent it.
        response = "\nMovies Collection:\n"
        for movie in self.collection:
            response += str(movie) + "\n"
        return response

f = Movies()

f.addMovieToCollection("Iron Man 1", "blue sign", 1995)
f.addMovieToCollection("Iron Man 2", "+7", 1995)
f.addMovieToCollection("Iron Man 1", "blue sign", 1996)
f.addMovieToCollection("Iron Man 1", "blue sign", 1995)
print(f)

output:

Added Iron Man 1 1995
Added Iron Man 2 1995
That movie likeIron Man 1 already exists
That movie likeIron Man 1 already exists

Movies Collection:
Movie name: Iron Man 2, Year: 1995
Movie name: Iron Man 1, Year: 1995
Iron Man 11995

Finally to print the movies I would advise you implement __str__ method in both classes. Movie class __str__ should return 1 movie. and Movies class __str__ should return the list of movie.

Hope this helps.

Vikash Singh
  • 13,213
  • 8
  • 40
  • 70
0

I thinks this will resolve your issue. Let me know if you meant something else.

class Movie:
    def __init__(self, name, pegi, year):
        self.name = name
        self.pegi = pegi
        self.year = year

class Movies(Movie):

    def __init__(self):
        self.collection = []

    def addMovieToCollection(self, name, pegi, year):
        super().__init__(name, pegi, year)
        structure_of_movie = name + " - " + pegi + " - " + str(year)
        #I removed the for loop here
        if structure_of_movie in self.collection : #IChanged this if condition as well
            print("That movie like" + self.name + " already exists")
        else:
            self.collection.append(structure_of_movie)

    def showMovie(self):
        return print(*self.collection, sep='\n')

f = Movies()

f.addMovieToCollection("Iron Man 1", "blue sign", 1995)
f.addMovieToCollection("Iron Man 1", "blue sign", 1995)
f.addMovieToCollection("Iron Man 2", "+7", 1995)
f.addMovieToCollection("Iron Man 3", "+16", 1995)
f.addMovieToCollection("Iron Man 4", "+12", 1995)
f.addMovieToCollection("Iron Man 5", "+18", 1995)
f.showMovie()
Sachin Rajput
  • 238
  • 7
  • 26
  • 1
    Not a downvoter. But when you posting answer you should explain how your answer solves the OP problem – deadshot Nov 28 '20 at 09:00
  • Thanks for the info but i did mention the changes in the code and the rest was same code that i used which was there in the question. sow what else was needed to explain...just help me with this one more piece of info – Sachin Rajput Nov 28 '20 at 09:23