lastConnectionInRow inconsistent with lastConnectionInStack

[BeksOmega]

Describe the bug

lastConnectionInStack lives on the block, while lastConnectionInRow lives on the connection.

Expected behavior

Connection manipulation functions should be stored together. This makes them easier to use. And it makes it less likely that they will be accidentally duplicated.

Additional context

Both of these functions are package, so they can be safely moved.

In addition to the two above location options (connection or block) I would like to raise the option of moving these functions into their own "connection manipulation" file, as they don't access any private state. (lastConnectionInRow isn't even on the prototype)

Todo link

[rachel-fenichel]

+1 to moving them into their own file.

[rachel-fenichel]

Are there any other functions that you would also move there?

[BeksOmega]

Are there any other functions that you would also move there?

If I wanted to really shuffle things around (and def break people hehe) I'd probably do something like this:

• Connection & RenderedConnection become simple data classes. All logic functions get moved.
• A new "ConnectionUtils" class/file is added which handles finding the closest connection, seeing if connections are compatible, etc. It would also contain the lastConnectionInRow and lastConnectionInStack functions.
• ConnectionDB gets changed to a generic collection which sorts based on coordinates. It basically stays the same except searchForClosest now accepts a callback instead of calling isConnectionAllowed directly.

If I didn't want to go crazy (which is probably a good idea hehe) You could maybe move the other functions that live directly on Connection.

• singleConnection_
• connectReciprocally_

But connectReciprocally_ should probably just be killed as it only gets used in one place, as far as I can tell (sorry buddy).

[rachel-fenichel]

Okay, that sounds too big for right now (as you expected).

You're making good points about inconsistencies in code structure, but I don't think it's worth it to move this code around at this point. We don't gain enough to pay for the churn.