HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

General pathfinder in Python

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
pathfinderpythongeneral

Problem

I was recently working on a project where I had the following setup:

```
from stargaze.sg import SGObject
from stargaze.constants import NegInf, Infinite, PATH_CLAMPED, PATH_REVERSIBLE, PATH_ACCELERATABLE
from stargaze.utils import clamp

__all__ = [
'PathIdentifier', '_basePath', '_clampedPath', '_reversiblePath',
'_acceleratablePath', 'Path'
]

class PathIdentifier(SGObject):
"""
INTERNATL: The base identifier class for a class object.
"""
OBJ_REPR_PREFIX = "StarGaze Path:: "

class _basePath(PathIdentifier):
def __init__(self, **config):
self._time = 0
self.speed = config['speed']

def __move__(self):
self._time += self.speed

class _clampedPath(PathIdentifier):

def __init__(self, **config):
start = config['start']
end = config['end']
if start > end:
errmsg = "Start point cannot be greater " \
"than the end point."
raise ValueError(errmsg)
self._time = start
self.end = end

self.finished = False

def __move__(self):
if self.finished:
return

if self._time >= self.end:
self.finished = True

class _reversiblePath(PathIdentifier):

def __init__(self, **config):
self.direction = 1

def __move__(self):
if self.direction == 1 and self._time >= 1:
self.direction = -1
self.speed *= -1
elif self.direction == -1 and self._time <= 0:
self.direction = 1
self.speed *= -1

class _acceleratablePath(PathIdentifier):

def __init__(self, speed, acceleration, min_speed=NegInf, max_speed=Infinite):
def __init__(self, **config):
self.accel = config['acceleration']

self.min_speed = config.get('min_speed', NegInf)
self.max_speed = config.get('max_speed', Infinite)

def __move__(self):
if self.accel != 1:
self.speed = clamp(self.accel * self.speed,

Solution

You are using classes as function containers. Path doesn't inherit from any of the other classes, yet you are passing your self instance to their initializer functions.

class _basePath:
    def __init__(self, **config):
        #Initialize Here
class Path:
    def __init__(self, flags, **config):
        _basePath.__init__(self, **config)


is no different from

def _base_path_init(instance, flag **config):
        #Initialize Here
class Path:
    def __init__(self, flags, **config):
        _base_path_init(self, **config)


You do the same thing in move() to access the specific move implementations.

If you take a step back, this code looks like it is trying to combine the concept of an object model with the concept of mixins or traits. Instead, if we take a step back and focus on the object model concept, the code can be simplified.

class Path(SGObject):
  def __init__(self, flags, **configs):
    SGObject.__init__(self)
    self._traits = []
    if flags & PATH_CLAMPED:
        self._traits.append(ClampedPath(self, **config))
    if flags & PATH_REVERSED:
        self._traits.append(ReversiblePath(self, **config))
    if flags & PATH_ACCELERATABLE:
        self._traits.append(AcceleratablePath(self, **config))

  def move(self):
    for trait in self._traits:
      trait.move(self)


Then each model would have the following interface.

class PathTrait(object):
  def __init__(self, instance, **configs):
     #Initialize Instance Here
  def move(instance):
     #Alter Instance Here


Notes:

The above assumes that the order of move operations needs to be enforced by Path (ie. ClampedPath's move must occur before ReversiblePath's move). If this is not the case, the initializer can be simplified to just take a list of model classes.

class Path(SGObject):
  def __init__(self, traits, **configs):
    SGObject.__init__(self)
    self._traits = [t(self, **config) for t in traits]


This allows you more flexibility in the future and removes the needs for flags entirely.

The way to code is written now, _clampedPath introduces the concept of movement no longer being allowed. If the path is clamped and it has finished, should the other traits execute at all?

Code Snippets

class _basePath:
    def __init__(self, **config):
        #Initialize Here
class Path:
    def __init__(self, flags, **config):
        _basePath.__init__(self, **config)
def _base_path_init(instance, flag **config):
        #Initialize Here
class Path:
    def __init__(self, flags, **config):
        _base_path_init(self, **config)
class Path(SGObject):
  def __init__(self, flags, **configs):
    SGObject.__init__(self)
    self._traits = []
    if flags & PATH_CLAMPED:
        self._traits.append(ClampedPath(self, **config))
    if flags & PATH_REVERSED:
        self._traits.append(ReversiblePath(self, **config))
    if flags & PATH_ACCELERATABLE:
        self._traits.append(AcceleratablePath(self, **config))

  def move(self):
    for trait in self._traits:
      trait.move(self)
class PathTrait(object):
  def __init__(self, instance, **configs):
     #Initialize Instance Here
  def move(instance):
     #Alter Instance Here
class Path(SGObject):
  def __init__(self, traits, **configs):
    SGObject.__init__(self)
    self._traits = [t(self, **config) for t in traits]

Context

StackExchange Code Review Q#51664, answer score: 3

Revisions (0)

No revisions yet.