Remove '.value' behaviour
We are going to simplify our library for the users by assuming that the attributes of our classes instances will always receive python basic types (int, string or list, when appropriate) or other complex objects (such as a Header object as message attributes).
Questions:
- What to do when dealing with our "Enums" os "Bitmasks"? Consider them "complex structures (objects)"?
- What to do with basic types such as "IPAddress" and "HWAddress"? We will accept "only" strings, or should the user add a HWAddress instance to that attribute?
My 2 cents:
- Enums and BitMasks: I prefer to treat them like a "complex structure" because the user would use it like i instead of ii:
- Store enum as a "complex structure":
if myclass.myenum == MyEnum.NAME- Store enum's value:
if myclass.myenum == MyEnum.NAME.value - IPAddress and HWAddress: these classes were created to translate human representation to binary, so I would let them hidden from the user and accept only strings.
Just some thoughts about the problem:
I believe we could have a more high-level interface in mind. There are 2 different ways to approach the issue, one that I call more 'open', and one that I call 'conservative'.
I prefer the 'conservative' one as it is less tolerant to mistakes and eases the validation process.
The 'open' one leaves too many possibilities, and makes maintenance and
debugging more difficult.
Some 'bad' uses like StatsRequest(body_type=1, body=body) area allowed
in the 'open' or in the current approach, but would be forbidden
in the 'conservative' one.
'Utility' (if we could call it that way) classes like the BitMask or the
HWAdress would be hidden from the user as UBInt8 for example (see also
https://github.com/kytos/python-openflow/issues/198).
Bellow follows what the received parameters could be in each case:
In the 'conservative' approach:
for Enums:
- a string, representing the attribute name of the Enum class:
+ 'OFPST_FLOW' (or simply 'FLOW')
for BitMasks:
- a list of strings representing the attribute names:
+ ['OFPC_FLOW_STATS', 'OFPC_PORT_STATS']
(or simply ['FLOW_STATS', 'PORT_STATS'])
for IPAdress and HWAdress:
- a string representing the address:
+ '00:00:00:00:00:00'
In the 'open' approach (many possibilities...):
for Enums:
- the used Enum class atribute:
+ StatsTypes.OFPST_FLOW
- a string, representing the attribute name of the Enum class:
+ 'OFPST_FLOW'
- the integer value of the flag:
+ 1
for BitMasks:
- a list of the used BitMask class attributes:
+ [Capabilities.OFPC_FLOW_STATS, Capabilities.OFPC_PORT_STATS]
- a list of strings representing the attributes:
+ ['OFPC_FLOW_STATS', 'OFPC_PORT_STATS']
- a list of the values:
+ [1, 4]
- an integer representing the bitmask:
+ 5
for IPAddress and HWAddress:
- a HWAdress instance
- a string representing the address:
+ '00:00:00:00:00:00'
- a tuple representing the address:
+ (0, 0, 0, 0, 0, 0)
My suggestion is to keep the values as the user assigned (e.g. bitmask as list of enums) so the user won't see a different value when accessing the attribute later. With this in mind, I liked:
- a list of the used BitMask class attributes:
[Capabilities.OFPC_FLOW_STATS, Capabilities.OFPC_PORT_STATS]- Now, we force the logic "or" operator (
|) that requires more effort to convert from bitmask to options list.
- Now, we force the logic "or" operator (
- (as already discussed before) for IPAdress and HWAdress:
- a string representing the address:
'00:00:00:00:00:00'
- a string representing the address:
@diraol @cemsbr @erickvermot I'm moving this to b3. We have to discuss better.
Agreed.