Any way to refactor this?

[John Krukoff]

Bruno Desthuilliers wrote:
> John Salerno a écrit :
> > Setting aside, for the moment, the utility of this method or even if
> > there's a better way, I'm wondering if this is an efficient way to do
> > it. I admit, there was some copying and pasting, which is what prompts
> > me to ask the question. Here's the method. (I hope it looks ok, because
> > it looks really weird for me right now)
> >
> > def _create_3D_xhatches():
> >     for x in xrange(-axis_length, axis_length + 1):
> >         if x == 0: continue
> >         visual.cylinder(pos=(x,-hatch_length,0),
> > axis=(0,hatch_length*2,0), radius=hatch_radius)
> >         visual.cylinder(pos=(x,0,-hatch_length),
> > axis=(0,0,hatch_length*2), radius=hatch_radius)
> >         visual.cylinder(pos=(-hatch_length,x,0),
> > axis=(hatch_length*2,0,0), radius=hatch_radius)
> >         visual.cylinder(pos=(0,x,-hatch_length),
> > axis=(0,0,hatch_length*2), radius=hatch_radius)
> >         visual.cylinder(pos=(-hatch_length,0,x),
> > axis=(hatch_length*2,0,0), radius=hatch_radius)
> >         visual.cylinder(pos=(0,-hatch_length,x),
> > axis=(0,hatch_length*2,0), radius=hatch_radius)
> >
> > Since each call to cylinder requires a slightly different format, I
> > figured I had to do it this way.
> 
>  From a purely efficiency POV, there are some obviously possible
> improvements. The first one is to alias visual.cylinder, so you save on
> lookup time. The other one is to avoid useless recomputation of
> -hatch_length and hatch_length*2.
> 
> def _create_3D_xhatches():
>      cy = visual.cylinder
>      for x in xrange(-axis_length, axis_length + 1):
>          if x == 0: continue
>          b = -hatch_length
>          c = hatch_length*2
>          cy(pos=(x, b, 0), axis=(0, c, 0), radius=hatch_radius)
>          cy(pos=(x, 0, b), axis=(0, 0, c), radius=hatch_radius)
>          cy(pos=(b, x, 0), axis=(c, 0, 0), radius=hatch_radius)
>          cy(pos=(0, x, b), axis=(0, 0, c), radius=hatch_radius)
>          cy(pos=(b, 0, x), axis=(c, 0, 0), radius=hatch_radius)
>          cy(pos=(0, b, x), axis=(0, c, 0), radius=hatch_radius)
> 
> A second step would be to try and generate the permutations by code
> instead of writing them all by hand, but I suppose the order is
> significant...
> There's still an obvious pattern, which is that the  position of 'c' in
> the axis tuple mirrors the position of 'b' in the pos tuple. There might
>   be some way to use this to let the computer handle some part of the
> repetition...
> 
> My 2 cents...
> --
> http://mail.python.org/mailman/listinfo/python-list

Because it was fun, and the previous refactoring made the pattern easy to
see, here's the way not to do this. (Requires python 2.5 for trinary
operator)

# From the ASPN cookbook
def permutations(L):
    if len(L) <= 1:
        yield L
    else:
        a = [L.pop(0)]
        for p in permutations(L):
            for i in range(len(p)+1):
                yield p[:i] + a + p[i:]

def _create_3D_xhatches():
    for x in xrange(-axis_length, axis_length + 1):
        if x <> 0: 
            # Use None as placeholder for comparison.
            for pos in permutations([x, None, 0]):
                visual.cylinder(pos = [-hatch_length if coord is None else
coord for coord in pos], axis = [hatch_length * 2 if coord is None else 0
for coord in pos], radius = hatch_radius)

Undoubtedly slower, nearly impossible to read, but has minimum of code
duplication! 

If I was answering this question for real, I'd suggest the same arguments as
tuple solution that was suggested earlier, i.e.:

def _create_3D_xhatches():
    list_of_kwargs = [
        {pos:[ x, -hatch_length, 0], axis:[0, 2 * hatch_length, 0 ]},
        {pos:[ x, 0, -hatch_length], axis:[0, 0, 2 * hatch_length ]},
        And so on for all permutations...
    ]

    for x in xrange(-axis_length, axis_length + 1):
        if x == 0: 
            continue

        for kwargs in list_of_kwargs:
            visual.cylinder( radius = hatch_radius, **kwargs )
    
As it pulls out the two obviously common components, the function call and
the radius parameter.
---------
John Krukoff
jkrukoff at ltgc.com