-1

Houses and cars have almost nothing in common for instance "number of rooms" and "gearbox". But if you live in an RV then I suppose it makes sense to have both. My problem is that I have a main data model named "Ad" for online classified advertisements and now we are going to expand the section about cars. Should I then extend the Ad object with CarAd(Ad) inheriting from Ad or should I just add fields to the original Ad class?

I apologize if the code below is dirty but it is the real thing.

class Ad(db.Model): #change to ndb.model
    cities = db.ListProperty(db.Key) #ndb.KeyProperty(repeated=True)
    regions = db.ListProperty(db.Key) #ndb.KeyProperty(repeated=True)
    blobs = db.ListProperty(db.BlobKey)  #ndb.BlobProperty(repeated=True)
    videos = db.ListProperty(db.BlobKey)
    hasvideo = db.BooleanProperty(default=False,
                                   verbose_name='has_video')
    primary_image = blobstore.BlobReferenceProperty()
    usr = db.ReferenceProperty()  # ndb_model.KeyProperty()
    ndb_usr = ndb.KeyProperty()
    hasimages = db.BooleanProperty(default=False,
                                   verbose_name='has_images')
    trending = db.BooleanProperty(default=False,
                                   verbose_name='trending')
    userID = db.StringProperty(verbose_name='User ID')
    integer_price = db.IntegerProperty()
    ip = ndb.StringProperty(verbose_name='ip')
    ipcountry = db.StringProperty(indexed=False, verbose_name='origin')
    tags = db.ListProperty(db.Category)
    category = db.CategoryProperty(verbose_name='Category')
    title = db.StringProperty(verbose_name='title')  # required
    type = db.StringProperty(verbose_name='ContentType')  # sell,wanted,rent,lease,buy
    company_ad = db.BooleanProperty(default=False,
                                    verbose_name='company_ad')  # false or nothing
    user = db.UserProperty(verbose_name='userid')
    im = db.IMProperty(verbose_name='nickname')  # optional, xmpp
    city = db.StringProperty()  # postaladdress should work instead
    region = db.StringProperty()  # postaladdress should work instead
    url = db.StringProperty(verbose_name='url')
    geopt = db.GeoPtProperty(verbose_name='geopt')
    text = db.TextProperty(verbose_name='text')
    currency = db.StringProperty(choices=(
        'INR',
        'EUR',
        'ARS',
        'AUD',
        'BRL',
        'GBP',
        'CAD',
        'CZK',
        'DKK',
        'HKD',
        'HUF',
        'ILS',
        'JPY',
        'MXN',
        'NZD',
        'NOK',
        'PLN',
        'PHP',
        'SGD',
        'SEK',
        'SGD',
        'CHF',
        'USD',
        'THB',
        'TWB',
    ), verbose_name='Currency')
    price = db.StringProperty(verbose_name='price')
    phonenumber = db.PhoneNumberProperty(indexed=False,
                                         verbose_name='phonenumber')  # viewbit
    phoneview = db.BooleanProperty(default=False,
                                   verbose_name='phoneview')
    email = db.EmailProperty(verbose_name='Email')  # optional
    name = db.StringProperty(verbose_name='Name')
    published = db.BooleanProperty(default=True,
                                   verbose_name='published')
    address = db.StringProperty(verbose_name='address')
    number_of_rooms = db.IntegerProperty()
    size = db.FloatProperty()
    regdate = db.IntegerProperty()
    mileage = db.IntegerProperty()
    added = db.DateTimeProperty(verbose_name='added', auto_now_add=True)  # readonly
    modified = db.DateTimeProperty(verbose_name='modified',
                                   auto_now_add=True)
    last_modified = db.DateTimeProperty(required=True, auto_now=True)
    crypted_password = db.StringProperty()  # set default true random
    salt = db.StringProperty()  # merge with passwrd, set default true random or why even store?
    timestamp = db.DateTimeProperty(auto_now=True)  #
    image_url = db.URLProperty();

    def __unicode__(self):
        return self.title

    def to_json(self):
        data = {}
        for prop in self.properties().values():
            data[prop.name] = prop.get_value_for_datastore(self)
        return json(data)

    def __encrypt(self, plaintext, salt=''):
        """returns the SHA1 hexdigest of a plaintext and salt"""

        phrase = hashlib.sha1()
        phrase.update('%s--%s' % (plaintext, salt))
        return phrase.hexdigest()

    def set_password(self, new_password):
        """sets the crypted_password"""

        if not self.salt:
            self.salt = self.__encrypt(str(datetime.now()))
        self.crypted_password = self.__encrypt(new_password, self.salt)

    def set_geography(self):
        """sets the ad's region and city"""

        url = 'http://maps.googleapis.com/maps/api/geocode/json' \
              + '?latlng={},{}&sensor=false'.format(self.geopt.lat,
                                                    self.geopt.lon)
        result = urlfetch.fetch(url)
        jsondata = json.loads(result.content)

        for result in jsondata['results']:
            for component in result['address_components']:
                if 'administrative_area_level_1' in component['types']:
                    self.region = component['long_name'
                    ].replace('County', '')
                if 'locality' in component['types']:
                    self.city = component['long_name']
        if self.city and self.region:
            self.place = self.city + ', ' + self.region
        elif self.city:
            self.place = self.city
        elif self.region:
            self.place = self.region

    def check_password(self, plaintext):
        return self.__encrypt(plaintext, self.salt) \
               == self.crypted_password

    def next(self):
        return (Ad.all().filter('published =', True).filter('modified >'
                                                            , self.modified).get() if Ad.all().filter('published ='
                                                                                                      , True).filter(
            'modified >',
            self.modified).get() else self)

    def prev(self):
        return Ad.all().filter('published =', True).filter('modified <'
                                                           , self.modified).get()

    def get_city(self):
        return montaomodel.City.get(self.cities[0])

    #def get_categoryname(self):
    #    return CATEGORIES[str(ad.category)]

    def get_adimage_serving_url(self):
        data = memcache.get('img-serv-url' + str(self.key().id()))
        if data is not None:
            return data
        else:
            if self.matched_images.get():
                try:
                    data = images.get_serving_url(str(self.matched_images.get().primary_image.key()), size=150).replace('http://', 'https://')
                except Exception, e:
                    logging.exception('exception: %s', str(e))
                    data = False
            else:
                data = False
            memcache.add('img-serv-url' + str(self.key().id()), data, 36000)
            return data

    def uri2view(self):
        try:
            return self.get_adimage_serving_url()
        except Exception, e:
            logging.exception('exception: %s', str(e))
            return None

    @classmethod
    def count_all(cls):
        """
        Count *all* of the rows (without maxing out at 1000)
        """

        count = 0
        query = cls.all().order('__key__')

        while True:
            current_count = query.count()
            if current_count == 0:
                return count
            count += current_count

            if current_count == 1000:
                last_key = query.fetch(1, 999)[0].key()
                query = query.filter('__key__ > ', last_key)

        return count
2
  • 2
    Wow, you really should be using an ORM for this sort of thing. Apr 1 '18 at 3:39
  • 1
    hmmm inheritance I think
    – Ewan
    Apr 1 '18 at 16:02
2

I would choose inheritance. Since you don't need to create a different advertising model for cars then using a child class will help this Ad class from getting anymore confusing. From what I see in your code, the original class encompasses data with different properties, and it was hard for me to follow the fields you use. I understand this is a production system, but I have to agree with Robert, using an object mapper would make it easier for you to see your data model more clearly. And I wonder if this would be easier to enhance and maintain if you had more classes. That way you can separate the advertising section from the vehicle information data. Or you could really pull them apart with Advertising, RV and Car/Vehicle classes

1

I suggest using decoration instead (not Python's decorators, but the Decorator design pattern), or just delegation. Otherwise, you'll eventually reach a point where the inheritance levels are so deep and the duplicate code is so ubiquitous that it's impossible to maintain.

Have a simple, generic model as a base, and then add such fields/ behaviors by decorating it.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.