0

I am implementing searching functions in Django, which of these would be better?

def same_cart_item_in_cart(cart, new_cart_item):
    already_exist_cart_item = cart.cartitem_set.filter(
        Q(variation__product=new_cart_item.variation.product),
        Q(variation=new_cart_item.variation),
        Q(width=new_cart_item.width),
        Q(height=new_cart_item.height),
    ).first()

    return already_exist_cart_item    # It can be None

Override CartItem's __eq__ first.

class CartItem(TimeStampedModel):
    cart = models.ForeignKey("Cart")
    variation = models.ForeignKey(Variation)
    # 벽화 너비
    width = models.PositiveIntegerField(
        default=1,
        validators=[MinValueValidator(1)],
    )
    # 벽화 높이
    height = models.PositiveIntegerField(
        default=1,
        validators=[MinValueValidator(1)],
    )
    quantity = models.PositiveIntegerField(
        default=1,
        validators=[MinValueValidator(1)],
    )

    class Meta:
        ordering = ('-created',)

    def __str__(self):
        return str(self.variation.product) + ' - ' + str(self.variation)

    def __eq__(self, other):
        return (
            self.variation.product == other.variation.product and
            self.variation == other.variation and
            self.width == other.width and
            self.height == other.height
        )

and then,

def same_cart_item_in_cart(cart, new_cart_item):
    for cart_item in cart.cartitem_set.all():
        if cart_item == new_cart_item:
            return cart_item

    return None
esote
  • 831
  • 12
  • 25
user3595632
  • 5,380
  • 10
  • 55
  • 111
  • Have you ran any time benchmarks? And i believe that query is the better choice(<1> option), because you need to make one query instead of retrieving all object and then iterate through each. – vishes_shell Sep 26 '16 at 21:27

1 Answers1

0

As i said in the comment <1> option is better.

And it you are trying to save new instance and check it before saving, Django made it for you. You can add unique_together to your Model Meta and you it will raise error if you would try to save partly same object.

UPDATE

It should optimize your code with using foregn key values directly

So your code would be 

def same_cart_item_in_cart(cart, new_cart_item):
    already_exist_cart_item = cart.cartitem_set.filter(
        Q(variation_id=new_cart_item.variation_id),
        Q(width=new_cart_item.width),
        Q(height=new_cart_item.height),
    ).first()

    return already_exist_cart_item

I removed Q(variation__product=...), if Variation instance is the same, you don't need to query on its fields. And changed Q(variation=...) to Q(variation_id=...)

vishes_shell
  • 22,409
  • 6
  • 71
  • 81
  • I use http://stackoverflow.com/a/1557584/3595632 and <1> is much faster! – user3595632 Sep 26 '16 at 21:35
  • `unique_together` would be great but have a problem. If same instance exist, I have to re-calculate the `quantity` like this : `instance.quantity += new_instance.quantity'... So that's why I couldn't use `unique_together`... Is there anything I can do for it? – user3595632 Sep 26 '16 at 21:38
  • @user3595632 i've updated answer, correct me if i'm wrong with penultimate sentence. – vishes_shell Sep 26 '16 at 21:56
  • I'm trying to do with `unique_together` but had some problems in catching the exception propery when same item exists.... Maybe I have to do with your method... – user3595632 Sep 26 '16 at 22:53
  • @user3595632 what seems to be the problem? It says `ValidationError` is raised when such record exists. I want it to be clear, do you still want the duplicate record to be saved? And how you set `unique_together`? I think it should be `unique_together = ('variation', 'width', 'height')` – vishes_shell Sep 26 '16 at 23:00
  • I set `unique_together = ('variation', 'width', 'height')`. But where should I catch the `ValidationError`? `clean()` or `full_clean()`? – user3595632 Sep 26 '16 at 23:32
  • Anyway, your answer do helps! – user3595632 Sep 26 '16 at 23:43
  • @user3595632 it should raise `ValidationError` when you are trying to `save()` it. So i think in your view or smth. – vishes_shell Sep 27 '16 at 06:47