On Fri, Nov 27, 2015 at 9:01 AM, Peter Karman <[email protected]> wrote:
> Nick Wellnhofer wrote on 11/26/15, 11:31 AM:
>>
>> Lucifers,
>>
>> If there's a method taking a bool parameter with a `true` default value:
>>
>> class Foo {
>> void
>> Bar(Foo *self, bool param = true);
>> }
>>
>> And we're passing `undef` to this method via Perl:
>>
>> $foo->bar(undef);
>>
>> Then the generated XS wrapper will convert `undef` to the default value
>> `true`. Since `undef` is often used as falsy value in Perl, this seems
>> wrong to me. For numbers, it makes sense to replace `undef` with a default
>> value, but we should make an exception for Booleans.
>
> +1 seems like the least surprising behavior.
-1
I see what Nick is getting at, but the rationale behind the current behavior
becomes clearer why when we consider named parameters. Furthermore, the
proposed behavior change will break Lucy code in the wild (including code in
Eventful's codebase written by my colleagues).
If undefined maps to "false", rather than the default, it becomes crucial to
differentiate between the following two cases:
1. a named parameter which was not supplied
2. a named parameter which was supplied but undef
Right now, something like this works:
# $config = { title => undef, content => undef };
my $config = read_config();
my $field_spec = Lucy::Plan::FullTextType->new(
analyzer => Lucy::Analysis::StandardTokenizer->new,
indexed => 1,
stored => $config->{$field}{stored},
sortable => $config->{$field}{sortable},
);
However, that example will break after the proposed behavior change, because
the interpretation of `stored` will change from "undef and therefore use the
default of true", to "undef and therefore false".
The workaround after the proposed behavior change would be to be very careful
about adding named parameters:
my %args = (
analyzer => Lucy::Analysis::StandardTokenizer->new,
indexed => 1,
)
if (defined $config->{$field}{stored}) {
$args{stored} = $config->{$field}{stored};
}
if (defined $config->{$field}{sortable}) {
$args{sortable} = $config->{$field}{sortable};
}
my $field_spec = Lucy::Plan::FullTextType->new(%args);
However, I wouldn't consider that an improvement, and it's not possible to
get there from where we are today without inducing silent failure.
I wouldn't rule out making a breaking change to Lucy for the sake of
Clownfish at some point in the future, but for this proposal, I don't think
the costs are justified by the benefits.
Marvin Humphrey
