Re: CVS update: subversion/subversion/tests/clients/cmdline svn_output.py

On Thu, Apr 12, 2001 at 11:59:30PM -0000, sussman@tigris.org wrote:
>...
> + elist, alist = expected_lines, actual_lines # copy lists so we can change 'em

That doesn't copy the lists. Names are simply references to objects. The
above two lines would be similar to this C program:

    struct some_object *expected_lines;
    struct some_object *actual_lines;
    struct some_object *elist;
    struct some_object *alist;

    elist = expected_lines;
    alist = actual_lines;

You're just copying pointers, not making copies.

Easiest way to copy a list is to take a slice of the whole list:

  elist = expected_lines[:]
  alist = actual_lines[:]

The "copy" module can copy just about anything, but no need to pull that in
other than for obviousness (seeing the word "copy" in the source makes a bit
clearer to those unfamiliar with the [:] idiom).

Not that performance matters, but I tend to do assignments one per line,
rather than using multiple assignments like you did above. Underneath, the
code does something like:

    temp_tuple = make_a_tuple(expected_lines, actual_lines)
    elist, alist = unpack_tuple(temp_tuple)

For swapping variables, the multiple assignment is perfect:

  a, b = b, a

It is clear, and the actual behavior works as expected.

But for assigning stuff to variables... I like one per line, unless you're
assigning one value to multiple items:

    a = b = 5

> + for eline in elist:
> + for aline in alist: # alist will shrink each time this loop starts
> + if not compare_lines(eline, aline, remachine):
> + del alist[alist.index(aline)] # safe to delete aline, because...

alist.remove(aline) is easier.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/