[Freeipa-devel] [PATCHES] 146-164 LDAP code refactoring (Part 4)

[Jan Cholasta]

On 1.2.2013 09:47, Petr Viktorin wrote:
> On 01/31/2013 07:01 PM, Jan Cholasta wrote:
>> On 31.1.2013 11:00, Petr Viktorin wrote:
>>> On 01/30/2013 10:53 AM, Petr Viktorin wrote:
>>>> On 01/29/2013 04:39 PM, Petr Viktorin wrote:
>>>>> On 01/28/2013 04:09 PM, Petr Viktorin wrote:
>>>>>> On 01/28/2013 09:34 AM, Jan Cholasta wrote:
>>>>>>> On 25.1.2013 14:54, Petr Viktorin wrote:
>>>>>>>> On 01/24/2013 03:06 PM, Petr Viktorin wrote:
>>>>>>>>> On 01/24/2013 10:43 AM, Petr Viktorin wrote:
>>>>>>>>>> On 01/22/2013 04:04 PM, Petr Viktorin wrote:
>>>>>>>>>>> On 01/21/2013 06:38 PM, Petr Viktorin wrote:
>>>>>>>>>>>> On 01/17/2013 06:27 PM, Petr Viktorin wrote:
>>>>>>>>>>>>> Hello,
>>>>>>>>>>>>> This is the first batch of changes aimed to consolidate our
>>>>>>>>>>>>> LDAP
>>>>>>>>>>>>> code.
>>>>>>>>>>>>> Each should be a self-contained change that doesn't break
>>>>>>>>>>>>> anything.
>>>>>>>>>>>>>
>>> [...]
>>>>>>>> Since this patchset is becoming unwieldy, I've put it in a public
>>>>>>>> repo
>>>>>>>> that I'll keep updated. The following command will fetch it into
>>>>>>>> your
>>>>>>>> "pviktori-ldap-refactor" branch:
>>>>>>>>
>>>>>>>>      git fetch git://github.com/encukou/freeipa
>>>>>>>> ldap-refactor:pviktori-ldap-refactor
>>>>>>>>
>>>>>>>>
>>> [...]
>>>
>>> I found a bug in patch 143, here is a fixed version.
>>>
>>
>> I would prefer if you used the semantics of .get() for .get_single() as
>> well (i.e. when no default value is provided, None is assumed) in patch
>> 152. Or is there a reason not to?
>
> I think you should always have to write extra code to supress exceptions
> (“Errors should never pass silently”). In Python, the easiest/shortest
> getter you can write will typically raise an exception when the value is
> missing (e.g. `d[k]` for dicts, `getattr(a, b)` for objects).
>

That is true, but I think consistency is more important here - the name 
suggests the method behaves the same way .get() does. If you insist on 
keeping the current behavior, would you at least consider renaming the 
method (perhaps to just "single" or "single_value")?

(This is just a nitpick, so don't worry too much about it.)

Honza

-- 
Jan Cholasta