On 01/ 5/10 10:01 PM, Keith Mitchell wrote:
> Hi Dermot,
>
> Dermot McCluskey wrote:
>> Keith,
>>
>> This is responding to all your remaining comments, except
>> unittesting.py, which I will leave Luis or Evan to discuss.
>>
>>
>>> 49-55: Is there any reason to not use direct attribute access?
>>
>> I think it's fairly standard to provide setter and getter
>> methods in cases like this. Also, it means the C and Python
>> user code is similar because there's a function to call
>> in both cases.
>
> The general case for Python is to start with direct attribute access,
> and transition the attribute to a "property" at a later time if
> getters/setters become necessary. At a minimum, you should start with
> the property syntax, so that there is only a single interface to mod_id
> (right now, consumers could use either self.mod_id or self.get_mod_id()).
>
> In the case of the C code, there are API functions for retrieving an
> attribute that should work. However, if they don't (or it's not easy to
> make them work in this case for some reason), using the property syntax
> would still be preferred for Python consumers.
I would be happy enough if we used the property syntax - with mod_id being a
read-only property, if nothing else it would prevent someone calling:
einfo.mod_id = "different_id"
which we don't want.
>
>>> 110-118: Could this be incorporated into the ErrorInfo.__init__? Or,
>>> is it a valid scenario to create an ErrorInfo independently from the
>>> list?
>>
>> I think it's neater to have a function to call here. And again,
>> it makes the C implementation easier by having a function to call.
>
> My concern here is that we have two seemingly identical paths for a
> Python consumer to create an error info that act in different ways. My
> personal preference is also to have code patterns match paradigms (e.g.,
> a Python consumer should be able to instantiate a class in the 'normal'
> way - in this case "my_err = ErrorInfo(...)") If having a 'helper'
> function to instantiate is easier for C consumers, then the function can
> stay, but please move line 117 into the __init__ code. (And if we need
> to be able to create ErrorInfos without adding them to the __errors
> list, than a True/False keyword argument to the __init__ function could
> default to adding to the list).
>
It wasn't intended that any consumer of this API should ever create an ErrorInfo
class - it should always be created using the specific function, since to do
otherwise it wouldn't be added to the list which is meant to contain all the
known ErrorInfo objects that exist.
But saying that, you are right, that it's more Python-like if we could allow the
constructor to be used in Python by moving line 117 into __init__() as:
__errors.append( self )
In that case, we could remove the create_error_info() function since it would
then only be of benefit to the C Consumer, but I don't see any harm in having
both options available for people to use.
Dermot, how difficult is it to create an ErrorInfo from the C API with the
specific arguments?
Thanks,
Darren.
