sourcebots / robot-api

(Legacy) API to interface with robotd
http://docs.sourcebots.co.uk/api/
MIT License
4 stars 1 forks source link

Keep initialisation in API consumers #7

Open PeterJCLaw opened 7 years ago

PeterJCLaw commented 7 years ago

In a number of places I've seen comments that there's a desire to move the API from:

from robot import Robot
bot_instance = Robot()

to something approximately like

from robot import bot_instance

I'd like to put a vote in for not doing this, as well as understand why this is seen as beneficial.

My experience is that libraries which try to be "helpful" by doing various initialisation for you under the covers tend to end up being difficult to use as soon as you want to do something interesting with them (including test them).

I think it's also the case that if we're teaching people to code by using this library as an example, we should keep this example as close to the other sorts of library they're likely to encounter.

There might be an argument for having a global robot instance as well as locally instantiable ones, with a pattern a bit like Python's re library:

import robot
robot.move_forward()

# you can also do
from robot import Robot
bot = Robot("custom", "stuff")
bot.move_forward()

# or maybe
bot = robot.initialise("custom", "stuff")
bot.move_forward()
# though this one is held back by lack of a decent name for the `initialise` function
RealOrangeOne commented 7 years ago

I too would vote against moving initialisation out of user space. Mostly because initialising by default would prevent advanced initialisation, e.g. any setup before wait_start

prophile commented 7 years ago

I would advocate for making the initialisation implicit—specifically initialising lazily, rather than initialising at import time.

Lazy initialisation:

The issues with the explicit creation of Robot that I see are threefold:

  1. It implies that it's possible to have multiple Robots. This is evidently untrue: it's very much a singleton and has always caused (sometimes quite tricky to debug) errors on subsequent calls.
  2. Competitors are often very new to programming and while we teach them functions, we don't generally teach them classes and we've often seen them confused by dot-notation. Avoiding OOP means it's possible to avoid the dot-notation entirely.
  3. Is heavily related to point 2: we're showing them a different syntax for calling our functions than calling their own. I'd rather opt for minimising the total amount of "magic" involved from the point of view of a competitor.

Yes, this means doing lazy initialisation which is a little ugly on our side, but from the point of view of the API as seen by a competitor this should be entirely an implementation detail. We don't ask them to call initialisation to be able to use the clock from time() from time, nor do we ask it for the ability to use the console from print()—why should it be any different for functions from robot?

PeterJCLaw commented 7 years ago

Is Robot a singleton per-process or over the whole machine? If the latter, then there really isn't a lot of difference whether or not the initialisation in the consumer as the API still needs to guard against other instances in other processes.

I think the lazy initialisation you're headed towards is approximately what I had in mind in my third example. I'd be in favour of primarily keeping the design of the API OOP (as it makes it nicer for us to use), but then also exposing a simpler interface for introductory use.

How about something which looks like:

from robot import simple_robot
simple_robot.move_forwards() # or whatever valid methods there are

The simple_robot might also only support a single power/servo/motor board:

>>> simple_robot.power_board.flash_led()
>>> simple_robot.power_boards[0].flash_led()
    Traceback
        ...
    AttributeError: simple_robot has no attribute 'power_boards'

Advanced users would still use the Robot class (which would offer support for multiple power boards, custom initialisation etc.) allowing them to opt-in to these features while keeping the surface area of the auto-lazy-initialised instance small.

A sketch of how the simple_robot might be defined:

# robot/simple_robot.py
from . import Robot

__all__ = (
    'power_board',
    'motor_board',
    ...
)

_instance = Robot()
for _method_name in __all__:
    locals()[_method_name] = getattr(_instance, _method_name)

This results in initialisation on import, though only for those who want the simple interface.