class variable stays empty with for loop

Question

I wanted to make sure that hotel_name doesn't exist in Hotel.hotels list. It seems that when I start feeding every time for loop look in an empty list.

Note if I am not using for loop and only do

Hotel.hotels.append([number,hotel_name,city,total_number,empty_rooms])

it prints hotels list

[[1, 'Crown Plaza', 'alex', 20, 2], [1, 'Crown Plaza', 'alex', 20, 2], [2, 'Radisson Blu', 'cairo', 24, 22], [3, 'Paradise Inn', 'dubai', 390, 200], [4, 'Four Seasons', 'alex', 1000, 400], [5, 'Address', 'dubai', 500, 200], [6, 'Fairmont', 'dubai', 1000, 100], [7, 'Rotana', 'dubai', 5000, 300]]
[Finished in 0.1s]

This is the file

class Hotel():
    """""""""
    this is hotel class file
    """
    hotels = []

    def __init__(self,number,hotel_name,city,total_number,empty_rooms):
        self.number = number
        self.hotel_name = hotel_name
        self.city = city
        self.total_number = total_number
        self.empty_rooms = empty_rooms
        for i in Hotel.hotels:
            if self.hotel_name not in i:
                Hotel.hotels.append([number,hotel_name,city,total_number,empty_rooms])
            # else:
            #     print "Hotel already exists!"

feed_dataBase_with_hotel = Hotel(1,"Crown Plaza","alex",20,2)
feed_dataBase_with_hotel = Hotel(1,"Crown Plaza","alex",20,2)# cull repeated
feed_dataBase_with_hotel = Hotel(2,"Radisson Blu","cairo",24,22)
feed_dataBase_with_hotel = Hotel(3,"Paradise Inn","dubai",390,200)
feed_dataBase_with_hotel = Hotel(4,"Four Seasons","alex",1000,400)
feed_dataBase_with_hotel = Hotel(5,"Address","dubai",500,200)
feed_dataBase_with_hotel = Hotel(6,"Fairmont","dubai",1000,100)
feed_dataBase_with_hotel = Hotel(7,"Rotana","dubai",5000,300)

print Hotel.hotels

Many Thanks to Patrick for his answer Update What if i want to build a list from the dict, as i want to access empty-rooms and change its value with another class

class Hotel():
    """
    this is hotel class file
    """
    hotels = {}
    hotelList = []

    def __init__(self,number,hotel_name,city,total_number,empty_rooms):
        self.number = number
        self.hotel_name = hotel_name
        self.city = city
        self.total_number = total_number
        self.empty_rooms = empty_rooms

        # edited: no for needed
        if self.hotel_name in Hotel.hotels:
            print('Hotel {} Already exists!'.format(self.hotel_name))
            return # or raise ValueError & handle it

        Hotel.hotels[self.hotel_name] = self

        tempList = Hotel.hotels.items()
        for i in tempList:
            x = Hotel.hotels.items()[i][1]
            Hotel.hotelList.append(x)

Update

another class for reservation will going to use the instance variable hotel_name which we are using in hotel class

from hotel import Hotel
from customer import Customer
from notification import Notification

class Reservation():
    reservations =[]
    def reserve_room(self,hotel_name, customer_name):
        x = Hotel.hotels.values()
        for i in x:
            if Hotel.hotel_name in i:
                Reservation.reservations.append([hotel_name,customer_name])
                i[4] -=1

AttributeError: class Hotel has no attribute 'hotel_name'

Update from Understanding getitem method using

def __getitem__(self, hotel_name):
          return self.hotel_name

issue solved!

Special Thanks to Patrick

Why do you create `Hotel` instances if you throw them away? You only ever store the last ``Hotel` in `feed_dataBase_with_hotel`. The `Hotel.hotels` only stores a simple list of the hotels attribute, _NOT_ the Hotel-Instance you just created. So you create and forget about it instantly. For a fix, see answer. — Patrick Artner, Jun 22 '18 at 06:21
@PatrickArtner Thanks for your answer , would you mind check my update? — Ahmed Younes, Jun 22 '18 at 18:36
I edited - the for loop is superflous - `if self.hotel_name in Hotel.hotels` checks _all_ keys already. The `tempList` and additionall `hotelList` is also superflous - instead of `dict.items()` which returns `(key,value)` tuples simply use `Hotel.hotels.values()` wich gives you all Hotel-Instances back, without the keys. hth — Patrick Artner, Jun 22 '18 at 19:07
Again thanks for the magnificent answers! feeling shy to say there is another last update please check it — Ahmed Younes, Jun 22 '18 at 20:55
your `i` is an `Hotel` instance - I think you should google a bit about instances and classes. your `x` are all the Hotel-Instances you got, if you `for i in x:` then `i` will be each single Hotel-Instance in turn. So to add the hotelname to reservations do: `Reservations.reservations.append([i.hotel_name, customer_name])` ... and please stop adding to this question as the "worth" decreases wich each question piled into it. Better to ask a new question. — Patrick Artner, Jun 22 '18 at 21:27
you also migth want to give [How to debug small programs (#1)](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/) a read. it teaches/describes how to debug your own code so you can come to fixes yourself. — Patrick Artner, Jun 22 '18 at 21:28
Will do it , thanks a lot , yeah i should do new questions forgive me im noob here — Ahmed Younes, Jun 22 '18 at 21:33

Patrick Artner · Accepted Answer · 2018-06-22T19:00:32.663

I would suggest changing some stuff around to fix it:

you are iterating a list of possible 1000 Hotels to find if one has the same name, it would be much better if you did that with a set or dict as lookups in those are O(1). Lists have O(n) lookup worst case time, meaning set/dict's are constant time, regardless of how many Hotels you've got. Lists get slower and slower, the more Hotels you need to search.
you overwrite the same variable with new Hotel-Instances, they themselfs get created and forgotten - you only store theire values inside your Hotel.hotels list instead of storing the constructed Hotel itself. The whole construction of Hotel instances is pointless.

Proposed changes:

make the static storage a dictionary which gives you the benefit of having fast lookup
store your Hotel-instances instead of the values that you created Hotels with
do not print Hotel.hotelDict directly - I introduced a method that takes only the values of your dicti and prints them sorted - as default I sort by Hotel.number

class Hotel():
    """""""""
    this is hotel class file
    """   
    hotelDict = {} # checking "in" for sets is much faster then list - a dict is similar
                   # fast in checking and can hold values - serves double duty here
    # sorted hotels by value, sorted by key

    @classmethod
    def getSortedHotels(cls, sortKey = lambda x:x.number):
        """Takes all values (Hotel's) from Hotel.hotelDict
        and prints them after sorting. Default sort key is Hotel.number"""
        return sorted(cls.hotelDict.values(), key=sortKey) 

    def __init__(self,number,hotel_name,city,total_number,empty_rooms):
        if hotel_name in Hotel.hotelDict:
            print("Hotel already exists: {}".format(hotel_name))
            return # or raise ValueError("Hotel already exists") and handle the error
# see https://stackoverflow.com/questions/3209233/how-to-replace-an-instance-in-init
# if you want to try to replace it using __new__() but not sure if its possible

        self.number = number
        self.hotel_name = hotel_name
        self.city = city
        self.total_number = total_number
        self.empty_rooms = empty_rooms
        Hotel.hotelDict[self.hotel_name] = self

    def __repr__(self):
        """Neater print output when printing them inside a list"""
        return "{:>3}) {} in {} has {} of wich {} are empty.".format(
        self.number,self.hotel_name,self.city,self.total_number,self.empty_rooms)
        # if you want a "simple" list-like output of your attributes:
        # comment the return above and uncomment:
        # return repr([self.number,self.hotel_name,self.city,
        #              self.total_number,self.empty_rooms])

    def __str__(self):
        """Neater print output when printing Hotel instances"""
        return self.__repr__()

Test with:

feed_dataBase_with_hotel = Hotel(1,"Crown Plaza","alex",20,2)
feed_dataBase_with_hotel = Hotel(1,"Crown Plaza","alex",20,2) # cull repeated
feed_dataBase_with_hotel = Hotel(2,"Radisson Blu","cairo",24,22)
feed_dataBase_with_hotel = Hotel(3,"Paradise Inn","dubai",390,200)
feed_dataBase_with_hotel = Hotel(4,"Four Seasons","alex",1000,400)
feed_dataBase_with_hotel = Hotel(5,"Address","dubai",500,200)
feed_dataBase_with_hotel = Hotel(6,"Fairmont","dubai",1000,100)
feed_dataBase_with_hotel = Hotel(7,"Rotana","dubai",5000,300)

print Hotel.getSortedHotels() 

print Hotel(99,"NoWayInn","NoWhere",1,200)

Output:

Hotel already exists: Crown Plaza
[  1) Crown Plaza in alex has 20 of wich 2 are empty.,   
   2) Radisson Blu in cairo has 24 of wich 22 are empty.,   
   3) Paradise Inn in dubai has 390 of wich 200 are empty.,   
   4) Four Seasons in alex has 1000 of wich 400 are empty.,   
   5) Address in dubai has 500 of wich 200 are empty.,   
   6) Fairmont in dubai has 1000 of wich 100 are empty.,   
   7) Rotana in dubai has 5000 of wich 300 are empty.]

 99) NoWayInn in NoWhere has 1 of wich 200 are empty.

If you want your hotels sorted by name, easy:

print Hotel.getSortedHotels(sortKey=lambda x:x.hotel_name)

hi Patrick Artner , Could you please explain ,What is purpose of using classmethod here. ? — PIG, Jun 22 '18 at 06:48
I would like to thank you for this answer since you answered it and I'm still reading it :D — Ahmed Younes, Jun 22 '18 at 15:16
Trying to underestand self = Hotel.hotelDict[hotel_name] i need your support Patrick .. Please — Ahmed Younes, Jun 22 '18 at 15:28
@PIG The dictionary that holds all the instances with the hotel_names as key is a class member (not an instance member, those are all `self.something`s). If you mark something as @classmethod and (by custom give it cls as 1st param) Python knows that you will be accessing class members in that method - you do not need a instance of a class to call this method, you can simply call it with `Hotel.getSortedHotels()` - no instance of Hotel required. You can however only access class members inside it (you could not use `self.hotel_name` f.e. because you do not have an instance. — Patrick Artner, Jun 22 '18 at 18:44
@PIG - see [meaning-of-classmethod-and-staticmethod-for-beginner](https://stackoverflow.com/questions/12179271/meaning-of-classmethod-and-staticmethod-for-beginner) — Patrick Artner, Jun 22 '18 at 18:45
@ahmedyounes it was a failed attempt to return the original created object, which does not work when already inside `__init__()` - forgot to edit it out, sorry. I edit it - you should simply return nothing or raise an valueError if you are ok with handling the error in the rest of your code. (See [how-to-replace-an-instance-in-init-with-a-different-object](https://stackoverflow.com/questions/3209233/how-to-replace-an-instance-in-init-with-a-different-object) ) -- Sorry for the caused confusion. — Patrick Artner, Jun 22 '18 at 18:58

score 0 · Answer 2 · answered Jun 22 '18 at 05:20

0

Take a look at this line for i in Hotel.hotels: here

for i in Hotel.hotels:
    if self.hotel_name not in i:
          Hotel.hotels.append([number,hotel_name,city,total_number,empty_rooms])

And try to get in which case it will fill your array

answered Jun 22 '18 at 05:20

Innuendo

631
5
9

score 0 · Answer 3 · answered Jun 22 '18 at 05:21

0

The problem is that you .append while iterating over an empty "hotels" list. Start with the check for existing hotel_name inside the for loop and append later.

for hotel in self.hotels:
    if self.hotel_name == hotel[1]:  # hotel_name is at index 1
        print('Already exists!')
        return  # return before appending
#now append
self.hotels.append([number,hotel_name,city,total_number,empty_rooms])

If you do not want to return, try this

for hotel in self.hotels:
    if self.hotel_name == hotel[1]:  # hotel_name is at index 1
        print('Already exists!')
        break
else:  # break did not happen
    self.hotels.append([number,hotel_name,city,total_number,empty_rooms])

answered Jun 22 '18 at 05:21

hoploop

44
2

and if i want to continue feeding after this return how can i do it ? – Ahmed Younes Jun 22 '18 at 05:28
with the break it will add the repeated hotel – Ahmed Younes Jun 22 '18 at 05:33
If break occurs, then else block is not fired. There has to be another bug in the code. Maybe filtering in the class does it's thing but you feed the database anyway? Try instantiating all Hotels and then iterating over Hotel.hotels calling "feed_dataBase_with_hotel". Why do you assign to this name as if it was a variable and not a function? – hoploop Jun 22 '18 at 05:46
@ahmedyounes Other thoughts: using a class as a simple filter seem to be unnecessary. A `dict` with hotel_names as keys or keeping a `set` of used names is likely a better approach. For more compact code, you could look at the `filter` function. – hoploop Jun 22 '18 at 05:57

class variable stays empty with for loop

3 Answers3