0

I've started learning python a few days ago, and I'm actually very impressed by its capabilities and syntax flexibility but today I encountered a strange bug that I've never seen in other programming languages, I suppose that it is caused by my limited knowladge of Python, I will appreciate any help and explanation of such behaviour.

I have a simple for loop in which I iterate over a list of nodes, in each iteration I add neighbours to current node, but it seems like they are added not just to current node but also to every other node in the collection, so at the end instead of having nodes with maximum 8 neighbours I end up having nodes with (8 * number of nodes in collection) neighbours, I have no idea what I missed here.

def evaluate_neighbours(nodes):
for node in nodes:
    node.neighbours.append(n for n in nodes if n.x == node.x - 1 and n.y == node.y)
    node.neighbours.append(n for n in nodes if n.x == node.x + 1 and n.y == node.y)
    node.neighbours.append(n for n in nodes if n.y == node.y - 1 and n.x == node.x)
    node.neighbours.append(n for n in nodes if n.y == node.y + 1 and n.x == node.x)
    node.neighbours.append(n for n in nodes if n.y == node.y + 1 and n.x == node.x + 1)
    node.neighbours.append(n for n in nodes if n.y == node.y + 1 and n.x == node.x - 1)
    node.neighbours.append(n for n in nodes if n.x == node.x - 1 and n.y == node.y + 1)
    node.neighbours.append(n for n in nodes if n.x == node.x + 1 and n.y == node.y - 1)

EDIT:

Node class and code that generates nodes is following:

class Node:
x = 0
y = 0
neighbours = []
alive = False

def __init__(self, _x, _y, _alive):
    self.x = _x
    self.y = _y
    self.alive = _alive


def generate_grid(data):
nodes = []
for index_y, y in enumerate(data):
    for index_x, x in enumerate(y):
        if x == "X":
            nodes.append(Node(index_x, index_y, True))
        else:
            nodes.append(Node(index_x, index_y, False))
return nodes
pablocity
  • 484
  • 4
  • 14
  • 2
    You need to show us how you created `nodes` so we can show you exactly how to fix it. But what's happened is that you _didn't_ create a bunch of separate `node` objects, you created a single one and you have multiple references to that one object in your `nodes` collection. – PM 2Ring Aug 25 '18 at 17:42
  • There a few ways that can happen [this question](https://stackoverflow.com/questions/240178/list-of-lists-changes-reflected-across-sublists-unexpectedly) talks about the most common way. – PM 2Ring Aug 25 '18 at 17:44
  • @PM2Ring: That question was my immediate thought on the cause. – ShadowRanger Aug 25 '18 at 18:31
  • 1
    There's also the classic `some_object_copy = some_object` fallacy. – juanpa.arrivillaga Aug 25 '18 at 18:32

1 Answers1

3

Your current code is appending a generator expression to the neighbors list. I'm pretty sure you want to be appending actual nodes, not generators. Further, since the generators are closures (don't worry too much if you don't know what this means), you may be doing your calculation wrong when it comes to deciding which nodes to append.

I'd suggest making a second explicit loop, rather than using any generator expressions, and turn all the if clauses you have in the generator expressions into parts of the condition in one if statement. That would look something like this:

for node in nodes:
    for n in nodes:
        if (n.x == node.x - 1 and n.y == node.y or
            n.x == node.x + 1 and n.y == node.y or
            ...):
                 node.neighbours.append(n)

I haven't copied over all the conditions, but you can do that, just connecting them with or. You might be able to group some of the conditions, if you wanted to streamline things (e.g. you could test n.x == node.x - 1 and node.y - 1 <= n.y <= node.y + 1 rather than using three different tests for the different y values).

Blckknght
  • 100,903
  • 11
  • 120
  • 169