List::BinarySearch and conditional tests with Test::More

glasswalk3r has asked for the wisdom of the Perl Monks concerning the following question:

Greetings monks,

I'm trying to use List::BinarySearch inside a SKIP: block of Test::More to implement conditional tests without success because I'm not being able to import the function binsearch with require.

This is what I'm trying to do:

SKIP: {

    eval { require List::BinarySearch; List::BinarySearch->import('bin
+search') };

    skip "List::BinarySearch not installed", 2 if $@;

    # a bunch of tests

}
[download]

Looking at the source of List::BinarySearch I found that it does inherits from Export and it is overriding the import (kind of weird because it is using goto instead calling SUPER) but somehow the method call is not working as expected because I'm getting those errors when executing:

Use of uninitialized value $a in string eq at t/010-sysinfo.t line 62.
Use of uninitialized value $b in string eq at t/010-sysinfo.t line 62.
Can't call method "binsearch" without a package or object reference at
+ t/010-sysinfo.t line 62.
[download]

I also tried using "List::BinarySearch::binsearch" but it just produce more errors:

Use of uninitialized value $a in string eq at t/010-sysinfo.t line 60.
Use of uninitialized value $b in string eq at t/010-sysinfo.t line 60.
Can't call method "List::BinarySearch::binsearch" without a package or
+ object reference at t/010-sysinfo.t line 60.
[download]

Am I missing something here, or should I contact the module author? :-)

Thanks,

Alceu Rodrigues de Freitas Junior
---------------------------------
"You have enemies? Good. That means you've stood up for something, sometime in your life." - Sir Winston Churchill

Comment on List::BinarySearch and conditional tests with Test::More Select or Download Code

Replies are listed 'Best First'.
Re: List::BinarySearch and conditional tests with Test::More by tobyink (Canon) on Nov 30, 2014 at 01:23 UTC
Don't call `import` because that only makes sense at compile-time. Call the `binsearch` function using its full name, and forcibly ignore its prototype... `SKIP: { eval { require List::BinarySearch } or skip "List::BinarySearch not installed", 2; $index = &List::BinarySearch::binsearch( sub {$a <=> $b}, 300, @num_array, ); # ... }` [download] Alternatively, split the tests which use `binsearch` into a separate test file, and in that file use: `use Test::Requires "List::BinarySearch";` [download] ... which will automatically do the right thing in terms of skipping, importing at compile time, etc. toby döt ink	[reply] [d/l] [select]
Re^2: List::BinarySearch and conditional tests with Test::More by davido (Cardinal) on Nov 30, 2014 at 07:01 UTC
That should be... `$index = &List::BinarySearch::binsearch( sub {$a <=> $b}, 300, \@num_array, );` [download] The difference from your original suggestion is that one should pass the array by reference. Q: Why would the crazy author want the array parameter to be passed by reference? Doesn't he know this can be inconvenient? :) A: Because passing a list is an O(n) operation, whereas passing a reference is O(1). We wouldn't want the passing of parameters to have an order of growth that is worse than the order of growth of the underlying algorithm. Of course you already know this, but it's worth mentioning in general. Dave	[reply] [d/l]
Re^2: List::BinarySearch and conditional tests with Test::More by glasswalk3r (Friar) on Nov 30, 2014 at 17:40 UTC
Thank you tobyink. Anyway, I thought that the idiom of calling a function with ampersand as something not used anymore. Doing a bit of reasearch I found this blog. As I'm always calling a function with "()" I never had any problem with that. Anyway... why the & operator is needed in this case? Since I already imported the module with `require` the code of the function should be already available... Alceu Rodrigues de Freitas Junior --------------------------------- "You have enemies? Good. That means you've stood up for something, sometime in your life." - Sir Winston Churchill	[reply] [d/l]
Re^3: List::BinarySearch and conditional tests with Test::More by tobyink (Canon) on Dec 01, 2014 at 07:51 UTC
Prototypes alter how a function call is parsed, but need to do so at compile time. So the interpretation of the function call depends on whether List::BinarySearch has already been loaded or not at the time your call to it is compiled. Because you don't know whether or not List::BinarySearch, it is safer to avoid the issue by bypassing prototypes (which is what the `&` does). toby döt ink	[reply] [d/l]
Re: List::BinarySearch and conditional tests with Test::More by davido (Cardinal) on Nov 30, 2014 at 07:10 UTC
"Am I missing something here, or should I contact the module author? :-)" Knowingly or not, you just did! :) tobyink is on the right track, though you should be passing the haystack array by reference if you override the prototypes because the prototype is `(&$\@)`, so pass by ref is what the function expects. ...I mention this in a followup to tobyink's otherwise perfect solution. As for the funky `import` override: I used goto &subroutine instead of calling `SUPER::subroutine()` because the former fixes up the call stack nicely so that Exporter::import never knows something else came first. But in retrospect, it probably wouldn't hurt to just call SUPER::.... Oh, and the reason that we're overriding `import` in the first place is because in Perl versions prior to 5.20 there was a problem where a single use of `$a` and `$b` in your script (except if affiliated with sort) would trigger some unwanted "used only once" warnings. This little import trick assured that `$a` and `$b` weren't used only once. ...an ugly hack. Come to think of it, I should probably only be overriding if I detect the Perl version predates 5.20, rather than the logic currently in place. So if you aren't importing anything, it is probably a good idea to put `no warnings 'once';` inside of the callback subroutine. If this seems even more ugly, it is... and it is not unique to List::BinarySearch either. List::Util and List::MoreUtils were suffering from the same nuisance as well, until Perl 5.20 came along, allowing `$a` and `$b` to become exempt from this warning. If you feel strongly enough that the override/goto code should be done differently please (and I mean this in all honesty), submit an issue at https://github.com/daoswald/List-BinarySearch/issues. Issues that include code, tests, and a strong argument are highly compelling to me. ;) At any rate, if this discussion doesn't lead you to a workable solution, please do post a GitHub issue so that it remains enough of an annoyance to me that I fix it. I despise leaving issues unresolved. :) Dave	[reply] [d/l] [select]
Re^2: List::BinarySearch and conditional tests with Test::More by glasswalk3r (Friar) on Nov 30, 2014 at 17:42 UTC
Fantastic davido... thank you for going through all the details. I'll check out if implementing those changes would avoid those errors and warnings without breaking anything and I'll let you know. Alceu Rodrigues de Freitas Junior --------------------------------- "You have enemies? Good. That means you've stood up for something, sometime in your life." - Sir Winston Churchill	[reply]
Re^3: List::BinarySearch and conditional tests with Test::More by davido (Cardinal) on Dec 01, 2014 at 16:58 UTC
For what it's worth, I put out a new version of List::BinarySearch today. The difference is subtle, and probably unimportant: In the previous version there was always a custom `import`, and that function did different things based on the Perl version number. In the new version there is only a custom import under versions of Perl that need it. The rest of the time we just accept what Exporter gives us. It still uses the `goto &...` idiom; calling through SUPER proved to be a little too fragile for our needs. Dave	[reply] [d/l] [select]


Perl: the Markov chain saw
	PerlMonks