t2mune/mrtparse

Replace `val_dict` by a better solution

raabf opened this issue · 3 comments

raabf commented

you seem to create the function val_dict only for the reason to return a default if the value is not in the dict. Without the default value the following lines are equivalent:

>>> MSG_ST[9][4]
'BGP_SYNC'
>>> val_dict(MSG_ST, 9, 4)
'BGP_SYNC'

This is a bit unpythonic and the actual meaning of the function is a bit hided. There are two better solutions for val_dict:

First assign the default value direct on accessing content. This has the advantage that the user see that it returns a default on Key Error, but it is more to write and not as nice as MSG_ST[9][4]:

>>> MSG_ST.get(9, dict()).get(4, 'unknown')
'BGP_SYNC'
>>> MSG_ST.get(9, dict()).get(90, 'unknown')
'unknown'
>>> MSG_ST.get(90, dict()).get(4, 'unknown')
'unknown'

If you want that behaviour in any case the best, nicest and cleanest solution would be to simple subclass the dict class:

>>> import collections
>>> BGP_ST = collections.defaultdict(lambda: "unknown", BGP_ST)
>>> BGP_ST[4]
'BGP_SYNC'
>>> BGP_ST[90]                                                                                                 
'unknown'

for dicts which returns dicts you have to use:

>>> MSG_ST = collections.defaultdict(lambda: dict(), MSG_ST)
>>> MSG_ST[9]                                                                                                          
{0: 'BGP_NULL', 1: 'BGP_UPDATE', 2: 'BGP_PREF_UPDATE', 3: 'BGP_STATE_CHANGE', 4: 'BGP_SYNC', 5: 'BGP_OPEN', 6: 'BGP_NOTIFY', 7: 'BGP_KEEPALIVE', 'BGP_KEEPALIVE': 7, 'BGP_NOTIFY': 6, 'BGP_NULL': 0, 'BGP_OPEN': 5, 'BGP_SYNC': 4, 'BGP_STATE_CHANGE': 3, 'BGP_UPDATE': 1, 'BGP_PREF_UPDATE': 2}
>>> MSG_ST[90]
{}

Of course the dics gets anyway obsolete with #5

Thank you!
These solutions are very nice!
We'll adopt second solution probably.

We replaced val_dict by using the collections module.
Thank you for your advice!
It became very helpful.

raabf commented

You're welcome. Glad to see that 😄