working on lists in python

Question

'm trying to make a small modification to django lfs project, that will allow me to deactivate products with no stocks. Unfortunatelly I'm just beginning to learn python, so I have big trouble with its syntax. That's what I'm trying to do. I'm using method 'is_variant' returning tru if my product is a sub type. If it is a variant I'm turning to parent product, get it's active variants and check their stocks. If stock is more than 0 variable active is 0, else it is 1. If after looping through variants 'active' is still 1 I set parent product's active to false.

I somehow cannot make it work the proper way. When using :

   def deactivate(self):
        if self.is_variant():
            prod = self.parent
            prod.active = all(var.get_stock_amount() != 0 for var in prod.variants.filter(active=True))
        else:
            prod.active = self.get_stock_amount() != 0

        self.parent.save()

It deactivates my product no matter if it's variants have stocks or not. And when using :

        inactive = 0
        if self.is_variant():
            prod = self.parent
            for s in prod.variants.filter(active=True):
                if s.get_stock_amount() == 0:
                    inactive = 1
                else:
                    inactive = 0
            if inactive == 1:
                prod.active = 0
            prod.save()
        else:
            if self.get_stock_amount() == 0:
                self.active = 0

            self.save()

The same happens, so my product is deactivated each time.

I've checked return types in shell and self is a variant and it is active.

Answer 1

First, I wouldn't call the list set, because this is a Python built-in method (see set). Use append on the list (your syntax is just incorrect and the error you get explicitly tells you so ;) ) and you have to initialize the list before:

def deactivate(self):
"""If there are no stocks, deactivate the product. Used in last step of checkout.
"""
if self.has_variants():
    sets = []
    for s in self.variants.filter(active=True):
        sets.append(s)  
    for var in sets:
        ...

But why creating a list beforehand if the only purpose is to iterate over it again? You can just do:

def deactivate(self):
"""If there are no stocks, deactivate the product. Used in last step of checkout.
"""
if self.has_variants():
    for s in self.variants.filter(active=True):   
        if s.get_stock_amount() == 0:
            inactive = True
        else:
            inactive = False
else:
    ...

Read more about lists.

Answer 2

This code is wrong in so many ways.

1. Creation of a list named set (as noted above)
2. Setting a variable multiple times in a loop without reading
3. Unneeded return value (if there is only one exit point, how can this be useful?)

I think this would work just as well:

def deactivate(self):
    """If there are no stocks, deactivate the product. Used in last step of checkout.
    """
    if self.has_variants():
        inactive = any(var.get_stock_amount() == 0 for var in self.variants.filter(active=True))
    else:
        inactive = self.get_stock_amount() == 0
    self.active = not inactive

or maybe:

def deactivate(self):
    """If there are no stocks, deactivate the product. Used in last step of checkout.
    """
    if self.has_variants():
        self.active = all(var.get_stock_amount() != 0 for var in self.variants.filter(active=True))
    else:
        self.active = self.get_stock_amount() != 0

Answer 3

Proper solution. Guess there's still plenty of place to optimize it but first I need to learn how :) :

def deactivate(self):
    """If there are no stocks, deactivate the product. Used in last step of checkout.
    """

    inactive = False

    if self.is_variant():
        prod = self.parent
        inactive = all(var.get_stock_amount() == 0 for var in prod.variants.filter(active=True))
        if inactive:
            prod.active = 0
        prod.save()
    else:
        if self.get_stock_amount() == 0:
            self.active = 0

        self.save()