Hungry ant AI: any food here?

Question

I'm working on a little AI to simulate ants (very basic). The full code can be found on Github. This function (in Ants.py) takes a list of Tuples containing coordinates and tells whether or not the ant is over one of them.

def Got_Food(self,all_foods):
    foods = []
    x,y = self.location[0],self.location[1]
    c_x,c_y = self.size[0],self.size[1]
    for food in all_foods:
          if food[1] >= y-c_y and food[1] <= y+c_y \
          and food[0] >= c_x and food[0] <=x+c_x:
            if food[1] > y and food[1] <y+c_y\
            and food[0] > x and food[0] <x+c_x:
                foods.append((food[0],food[1]))
            elif food[1] < y and food[1] >y-c_y\
            and food[0]  > x and food[0] >x-c_y:
                foods.append((food[0],food[1]))
          if food[0] >= x-c_x and food[0] <= x+c_x \
          and food[1] >= y and food[1] <=y+c_y:
                foods.append((food[0],food[1]))
    if len(foods) == 0:
        return None
    return foods

This function works exactly as I want, but it is probably one of the ugliest I've ever seen or written. Does anyone know a way around all these nested loops?

Answer 1

Disclaimer: Current version of the code seems to be very weirdly indented. Some comments might be wrong because of some misinterpretation of your code.

Style

Python has a style guide called PEP 8. It is usually a good idea to try to stick to it unless you have good reasons not to. In any case, it is definitly worth the read. You'll find various tools to check compliancy. In your case, the main issues would be : naming (snake_case is recommended for function names), spacing (whitespace around operators), indentation (4 spaces)

Design

Your function returns either a list of food or None is there is no food. This seems to be more complicated than it should be: it could return a list of food (empty is there is no food). It makes your function easier to use, easier to explain and more concise: you can simply remove the if len(foods) == 0: condition.

From the Zen of Python:

Special cases aren't special enough to break the rules.

In any case, your function deserves some documentation.

Tuple unpacking

One of my favorite feature of Python is iterable unpacking. You can do something like:

a, b, c = my_list_with_3_elements

and have easy access to the different elements (and an exception if the list has the wrong number of elements).

In your case, assuming location and size have the wrong length, you could simply write:

 x, y = self.location
 c_x, c_y = self.size

This is very convenient to avoid long expressions with error-prones bracket access to a particular index.

Going further, you could do the same with food[0] and food[1].

Now, (assuming I haven't broken too many things) looks like:

def got_food(self, all_foods):
   foods = []
   x, y = self.location
   c_x, c_y = self.size
   for f_x, f_y in all_foods:
      if f_y >= y - c_y and f_y <= y + c_y and f_x >= c_x and f_x <= x + c_x:
          if f_y > y and f_y < y + c_y and f_x > x and f_x < x + c_x:
              foods.append((f_x, f_y))
          elif f_y < y and f_y > y - c_y and f_x  > x and f_x > x - c_y:
              foods.append((f_x, f_y))
      if f_x >= x - c_x and f_x <= x + c_x and f_y >= y and f_y <= y + c_y:
            foods.append((f_x, f_y))
  return foods

Chained comparisons

From the doc:

Comparisons can be chained arbitrarily, e.g., x < y <= z is equivalent to x < y and y <= z, except that y is evaluated only once (but in both cases z is not evaluated at all when x < y is found to be false).

In your case, many things can be rewritten:

def got_food(self, all_foods):
   foods = []
   x, y = self.location
   c_x, c_y = self.size
   for f_x, f_y in all_foods:
      if y - c_y <= f_y <= y + c_y and c_x <= f_x <= x + c_x:
          if y < f_y < y + c_y and x < f_x < x + c_x:
              foods.append((f_x, f_y))
          elif y > f_y > y - c_y and f_x > x and f_x > x - c_y:
              foods.append((f_x, f_y))
      if x - c_x <= f_x <= x + c_x and y <= f_y <= y + c_y:
            foods.append((f_x, f_y))
  return foods

I do not understand enough of the intent to go any further (also I suspect the condition f_x > x is wrong).

Answer 2

I simplified you expressions and got the same as Josay. These are:

(1) y - c_y <= f_y <= y + c_y and     c_x <= f_x <= x + c_x
(2) y       <  f_y <  y + c_y and x       <  f_x <  x + c_x
(3) y - c_y <  f_y <  y       and x - c_y <  f_x            and x < f_x

(4) y       <= f_y <= y + c_y and x - c_x <= f_x <= x + c_x

I think you have three problems:

• (1) you use c_x which probably should be x.
• (3) x < f_x should probably be f_x < x.
• (3) You use c_y that should probably be c_x.

Fixing these results in:

(1) y - c_y <= f_y <= y + c_y and x       <= f_x <= x + c_x
(2) y       <  f_y <  y + c_y and x       <  f_x <  x + c_x
(3) y - c_y <  f_y <  y       and x - c_x <  f_x <  x      

(4) y       <= f_y <= y + c_y and x - c_x <= f_x <= x + c_x

If you merge (1) with (2), to make (5), and (1) with (3), to make (6). This will remove one of your ifs, and should result in:

(5) y       <  f_y <  y + c_y and x       <  f_x <  x + c_x
(6) y - c_y <  f_y <  y       and x       <  f_x <  x      

(6) can never be true, so it leaves us with:

(5) y       <  f_y <  y + c_y and x       <  f_x <  x + c_x
(4) y       <= f_y <= y + c_y and x - c_x <= f_x <= x + c_x

It should be quite apparent that they are almost the same equation. And so half the time you would add the same food twice rather than once.

---

From your very short description:

takes a list of Tuples containing coordinates and tells whether or not the ant is over one of them.

I would assume that you want to know if the ant, (x, y) is within +-(c_x, c_y) of the food (f_x, f_y). This is simply:

x - c_x <= f_x <= x + c_x and y - c_y <= f_y <= y + c_y

There is also a clear lack of documentation on what you want the function to do, and so I would add a docstring.

def Got_Food(self, all_foods):
    """
    Get food ant can find.

    If the food is within the square area that the ant can find food,
    food between self.location +- self.size.
    Then add it to the food list.

    This area looks like:

          f        S
      +-----+-----+
      |           |
      |        F  |
      +     A     +
      |           |
      |F          |
      +-----+-----+
    -S        f

    A = Ant's location
    S = Size that ant can find food
    F = Findable food
    f = Unfindable food
    """
    foods = []
    ...

Answer 3

The answer from @Josay is completely correct, those are some rules that should be considered for all Python code. The answer from @JoeWallis is excellent. The documentation using ASCII diagrams is what allowed me to understand what you were trying to do. From there, programming it was a joy. I think that the key concepts are three: factorise, generalise and abstract. Let's go by parts:

• Factorise: Left and right are pretty much the same. What you want to check is if a number is in a range, i.e. if the distance to the center is less than some number. That is - and abs
• Generalise: Horizontal and vertical are pretty much the same. In fact, you could have more than two dimensions, any number of dimensions. You just have to do the same operation for all dimensions.
• Abstract: If a piece of code gets complicated, just divide it in smaller chunks, it is easier to check whether one piece of food is __findable by the ant than finding out which pieces of food are close and which ones are not. If something looks complicated, chop it.

Considering these points, this is my proposal for implementation:

def __findable(self, food):
    return all(abs(l - food[i]) < self.size[i] for i, l in enumerate(self.location))

def got_food(self, foods):
    return [f for f in foods if findable(f)]

I didn't test it, "it should work". If it doesn't, I guess that the concepts and ideas are clear and fixing it should not be too hard. If the second line is too long for your taste (because we are at code review), then split it again:

def __in_range(self, foodd, dimension):
    return abs(self.location[dimension] - foodd) < self.size[dimension]

def __findable(self, food):
    return all(__in_range(f, i) for i, f in enumerate(food))

def got_food(self, foods):
    return [f for f in foods if findable(f)]

This doesn't look like much of an improvement. Similarly, you could put everything in a single function:

def got_food(self, foods):
    return [food for food in foods             \
        if all(abs(l - food[i]) < self.size[i] \
        for i, l in enumerate(self.location))]

All in all, my favourite is the first option, but this last one doesn't look too horrible to me.