How much sanity checking is required for function inputs?

[Chris Angelico]

On Wed, Apr 20, 2016 at 12:20 PM, Christopher Reimer
<christopher_reimer at icloud.com> wrote:
> I think you misread my code. No dictionary exception occurs in the sanity
> checks. Below is the full function with the revised sanity check for
> positions that compares the input list with the two valid lists of board
> positions.
>
>
> def generate_set(color, positions):
>
>     if positions not in [VARS['COORDINATES'][VARS['BOARD_BOTTOM']],
>                          VARS['COORDINATES'][VARS['BOARD_TOP']]]:
>         raise Exception("List for positions contains no valid coordinates, "
>                         "got {} instead.".format(positions))
>
>     # generate objects according to color and position
>     for position in positions:
>         rank, file = position
>         if rank in VARS['RANK_NOBILITY']:
>             if file in VARS['FILE_ROOK']:
>                 yield Rook(color, position)
>             elif file in VARS['FILE_BISHOP']:
>                 yield Bishop(color, position)
>             elif file in VARS['FILE_KNIGHT']:
>                 yield Knight(color, position)
>             elif file is VARS['FILE_QUEEN']:
>                 yield Queen(color, position)
>             elif file is VARS['FILE_KING']:
>                 yield King(color, position)
>         else:
>             yield Pawn(color, position)
>

If you nuke the check at the beginning, what happens? (By the way, I
don't like this shouty "VARS" thing you have going on. It makes it
hard to skim the code. I'm not entirely sure what your initial "not
in" check is even doing.) You seem to have two slightly different ways
of checking things: first something about BOARD_TOP and BOARD_BOTTOM,
and then something else about RANK_NOBILITY and... everything else. It
looks to me like all you need is to have a second check for
RANK_PAWNS, and then raise an exception if it's neither of those. Then
you don't need to pre-check, and the rules aren't split into two
places.

But if I were doing this, I'd make it far more data-driven.

starting_positions = {
    VARS['FILE_ROOK']: Rook,
    VARS['FILE_BISHOP']: Bishop,
    ...
}

Or, even better, tie that to the definitions of your classes, in some
way. (Metaprogramming is great here.) Then your code doesn't need an
if/elif tree; you can generate starting positions easily by just
looking up your table.

BTW, it's usually best to not raise Exception, but a custom subclass
of it. But I'm going to assume that this is just an example.

ChrisA