gvwilson / sdxpy

Software Design by Example: a tool-based introduction with Python
https://third-bit.com/sdxpy/
Other
353 stars 53 forks source link

/layout - remove spread operator and varargs? #216

Closed gitgithan closed 1 year ago

gitgithan commented 1 year ago

Throughout the files I see numerous appearances of spreading and collecting varargs.

class Row:
    def __init__(self, *children):

Row takes a comma separated list of inputs and collects into a tuple children

class PlacedRow(Row):
    def __init__(self, *children):
        super().__init__(*children)

To use Row, PlacedRow took a comma separated list of inputs, collected it, then immediately spread it just because Row expected a comma separated list of inputs.

Is this really necessary? How about removing all such stars and just letting the api pass/receive iterables directly instead of packing/unpacking repeatedly?

The only place i see a possible issue is class WrappedRow(PlacedRow):

    def wrap(self):
        children = [c.wrap() for c in self.children]
        rows = self._bucket(children)
        new_rows = [PlacedRow(*r) for r in rows]
        new_col = PlacedCol(*new_rows)
        return PlacedRow(new_col)

new_col is a PlacedCol that becomes (PlacedCol,) in PlacedRow's init. However this can easily be fixed with return PlacedRow([new_col]) assuming all stars were removed to expect pass/receive of iterables.

Side note:

from wrapped import WrappedBlock as Block
from wrapped import WrappedCol as Col
from wrapped import WrappedRow as Row

was confusing me for so long until i saw this block (didn't scroll all the up long test file while testing a specific function below) I thought Block, Col, Row were coming from easy_mode.py instead of the aliased ones from wrapped.py. Wish the namings don't clash to ease cognitive load.

gvwilson commented 1 year ago

Thank you for filing this issue:

  1. I'll have a look at getting rid of spreading with *, but I find myself using it a lot these days, and wanted to show it in context to help readers become familiar with it.
  2. I originally imported WrappedBlock etc. as-is, but people complained that was confusing as well. I think the real problem is that I'm deriving classes rather than overwriting old ones (which is what I'd do if I was developing this code for real), so I'm inevitably going to have a lot of similar names for things.