Code review comment for lp:~cr3/checkbox/xinput_resource

Revision history for this message
Marc Tardif (cr3) wrote :

* Zygmunt Krynicki <email address hidden> [2012-09-14 14:13 -0000]:
[snip]
> Actually I grew fond of purely-A ABCs as interfaces.
>
> from abc import ABCMeta, abstractmethod
>
> class IXInputResult:
> metaclass = ABCMeta
>
> @abstractmethod
> def addXinputDevice(self, device):
> """
> docs
> """
>
> The static part of you could even require isinstance(result,
> IXInputResult) but in general it's a simple way to ensure we understand
> the responsibilities of various classes without creating fat abstract
> base classes.

Following discussions on IRC, I just declared a base class with empty
functions and comments.

> (ps, any reason why you named it X{lowercase i}input and not XInput?

Same reason as the DmidecodeParser class; "dmidecode" and "xinput" are
command names that I consider as one word.

> Not really, frankly I needed to look-up a few of the syntax pieces here
> so maybe just explain that in a few # comments what the thing being
> parsed is. More usefulness than form or convention.

Thanks for the advice, that inspired me to also add a unit test for each
regular expression. I hope you like all the comments!

« Back to merge proposal