Re: Devel::Cover Problem: testing || for a default value.

2005-07-15 Thread Smylers
Michael G Schwern writes: > On Tue, Jul 12, 2005 at 07:45:35AM +, Smylers wrote: > > > A good way of putting assumptions into code is with (Michael's > > excellent) Carp::Assert: > > > > assert $p || $q, 'Either $p or $q must be supplied' if DEBUG; > > > While I get where you're going, I

Re: Devel::Cover Problem: testing || for a default value.

2005-07-12 Thread Michael G Schwern
On Tue, Jul 12, 2005 at 10:44:02PM -0400, James E Keenan wrote: > >The other examples in the ticket play out the same way: > > > > bless {}, ref $class || $class; > > > > I encountered the coverage problem inherent in this code in the > constructor of a module whose maintenance I recently ass

Re: Devel::Cover Problem: testing || for a default value.

2005-07-12 Thread James E Keenan
Michael G Schwern wrote: The other examples in the ticket play out the same way: bless {}, ref $class || $class; I encountered the coverage problem inherent in this code in the constructor of a module whose maintenance I recently assumed. (For that matter, I could have encountered

Re: Devel::Cover Problem: testing || for a default value.

2005-07-12 Thread David Golden
[EMAIL PROTECTED] wrote: I respectfully disagree. I think you're focusing too much on the low-level behavior of || returning one of its operands. That behavior makes Perl's syntax flexible and a little ambiguous. Because Perl doesn't make a distinction between "assign with a default value" and

Re: Devel::Cover Problem: testing || for a default value.

2005-07-12 Thread Shlomi Fish
On Monday 11 July 2005 23:17, Michael G Schwern wrote: > On Mon, Jul 11, 2005 at 07:38:57PM +0300, Yuval Kogman wrote: > > > So what should I do to eliminate it? > > > > Maybe Just Nothing > > > > The issue is that you can't special case get_current_coords to be > > truish, as far as Devel::Cover i

Re: Devel::Cover Problem: testing || for a default value.

2005-07-12 Thread mjcarman
David Golden wrote: > > I think this is a coverage vs correctness distinction. The idea that > I was trying to convey is that while these expressions use a boolean > operator for a shortcut, they aren't really about truth vs. falsity > of the overall expression, *except* when they are being used

Re: Devel::Cover Problem: testing || for a default value.

2005-07-12 Thread Smylers
Michael G Schwern writes: > "my $foo = $p || $q" is not boolean. I'm not even sure you can call > it "pseudo-boolean" without understanding the surrounding code. How > do you know that $q can never be false? So we want some way of annotating the code which will let Devel::Cover know that you're

Re: Devel::Cover Problem: testing || for a default value.

2005-07-12 Thread Michael G Schwern
On Tue, Jul 12, 2005 at 07:45:35AM +, Smylers wrote: > I can see why having 'inline' directives specifically for Devel::Cover > could be seen as ugly, but if instead they are general-purpose > assumptions then it's obviously better to have them next to the code > that's relying on the assumptio

Re: Devel::Cover Problem: testing || for a default value.

2005-07-11 Thread Michael G Schwern
On Mon, Jul 11, 2005 at 08:28:01PM -0600, Jim Cromie wrote: > 1. emit a text version of the coverage failures which > a. has same info as html details - ie line# conditionals, code > b. starts with a comment. > > 2. use that 'exact' format as your ignore directive > a. except that # means its enti

Re: [Maybe Spam] Re: Devel::Cover Problem: testing || for a default value.

2005-07-11 Thread leif . eriksen
[EMAIL PROTECTED] wrote: On Tue, 2005-07-12 at 10:46 +1000, [EMAIL PROTECTED] wrote: 1. Add code to handle the 'both false' case, similiar to my $class = ref $proto || $proto; warn 'wrong calling convention for Class::Constructor::new - try Class::Constructor->new' and return unles

Re: Devel::Cover Problem: testing || for a default value.

2005-07-11 Thread chromatic
On Tue, 2005-07-12 at 10:46 +1000, [EMAIL PROTECTED] wrote: > I'd say this idiom is one of the ones I am most often affected by in the > work I do for the Kwalitee project - the my$class = ref$proto||$proto; > idiom in constructors. I usually do the following > > 1. Add code to handle the 'both

Re: Devel::Cover Problem: testing || for a default value.

2005-07-11 Thread Jim Cromie
Paul Johnson wrote: On Mon, Jul 11, 2005 at 02:54:19PM -0700, Michael G Schwern wrote: On Mon, Jul 11, 2005 at 11:22:51PM +0200, Paul Johnson wrote: my $foo = $bar || default(); # DC ignore X|0 "Hey, Devel::Cover! Ignore the case where the right side of this logic is false.

Re: Devel::Cover Problem: testing || for a default value.

2005-07-11 Thread leif . eriksen
[EMAIL PROTECTED] wrote: Michael G Schwern wrote: you're right that the case of $class being false may be of interest, but that's not what this common idiom actually does. The code will blithely pass a false value to bless (with potentially unexpected results depending on whether $class

Re: Devel::Cover Problem: testing || for a default value.

2005-07-11 Thread Michael G Schwern
On Mon, Jul 11, 2005 at 07:43:24PM -0400, David Golden wrote: > I think this is a coverage vs correctness distinction. The idea that I > was trying to convey is that while these expressions use a boolean > operator for a shortcut, they aren't really about truth vs. falsity of > the overall expr

Re: Devel::Cover Problem: testing || for a default value.

2005-07-11 Thread David Golden
Michael G Schwern wrote: "my $foo = $p || $q" is not boolean. I'm not even sure you can call it "pseudo-boolean" without understanding the surrounding code. How do you know that $q can never be false? The other examples in the ticket play out the same way: bless {}, ref $class || $class; $cla

Re: Devel::Cover Problem: testing || for a default value.

2005-07-11 Thread Paul Johnson
On Mon, Jul 11, 2005 at 02:54:19PM -0700, Michael G Schwern wrote: > On Mon, Jul 11, 2005 at 11:22:51PM +0200, Paul Johnson wrote: > > > my $foo = $bar || default(); # DC ignore X|0 > > > > > > "Hey, Devel::Cover! Ignore the case where the right side of this logic is > > > false." > > > > I

Re: Devel::Cover Problem: testing || for a default value.

2005-07-11 Thread Michael G Schwern
On Mon, Jul 11, 2005 at 05:33:26PM -0400, David Golden wrote: > http://rt.cpan.org/NoAuth/Bug.html?id=11304 > > My suggestion turns on the question of whether it's possible to > differentiate the context between a true boolean context ( "foo() if $p > || $q" ) and a "pseudo-boolean" context that

Re: Devel::Cover Problem: testing || for a default value.

2005-07-11 Thread Yuval Kogman
On Mon, Jul 11, 2005 at 17:33:26 -0400, David Golden wrote: > Want.pm seems to imply that this might be possible, but I don't > know the guts of Perl well enough. Want looks into the opcode tree and looks for context thingies inside it, so it should be possible as far as I understand from readi

Re: Devel::Cover Problem: testing || for a default value.

2005-07-11 Thread Michael G Schwern
On Mon, Jul 11, 2005 at 11:22:51PM +0200, Paul Johnson wrote: > > my $foo = $bar || default(); # DC ignore X|0 > > > > "Hey, Devel::Cover! Ignore the case where the right side of this logic is > > false." > > I wasn't particularly happy with the idea of needing to change the > source code j

Re: Devel::Cover Problem: testing || for a default value.

2005-07-11 Thread David Golden
Michael G Schwern wrote: What's missing is a way to let Devel::Cover know that a bit of coverage is not necessary. The first way to do this which pops into my mind is a comment. my $foo = $bar || default(); # DC ignore X|0 I posted an item about this usage of the "||" operator th

Re: Devel::Cover Problem: testing || for a default value.

2005-07-11 Thread Paul Johnson
On Mon, Jul 11, 2005 at 01:17:48PM -0700, Michael G Schwern wrote: > On Mon, Jul 11, 2005 at 07:38:57PM +0300, Yuval Kogman wrote: > > > So what should I do to eliminate it? > > > > Maybe Just Nothing > > > > The issue is that you can't special case get_current_coords to be > > truish, as far as

Re: Devel::Cover Problem: testing || for a default value.

2005-07-11 Thread Yuval Kogman
On Mon, Jul 11, 2005 at 13:17:48 -0700, Michael G Schwern wrote: > On Mon, Jul 11, 2005 at 07:38:57PM +0300, Yuval Kogman wrote: > > I'll make the same argument "no broken windows" argument here that I do > about warnings and tests: elminate all warnings, even if they are dubious. > Ensure all

Re: Devel::Cover Problem: testing || for a default value.

2005-07-11 Thread Michael G Schwern
On Mon, Jul 11, 2005 at 07:38:57PM +0300, Yuval Kogman wrote: > > So what should I do to eliminate it? > > Maybe Just Nothing > > The issue is that you can't special case get_current_coords to be > truish, as far as Devel::Cover is concerned - it might not be. > > Any fix that could be thought u

Re: Devel::Cover Problem: testing || for a default value.

2005-07-11 Thread Jérôme Fenal
2005/7/11, Yuval Kogman <[EMAIL PROTECTED]>: > Coverage reporting is not done for the pretty colors - a human reads > it, and says "OK, this is logical, get_current_coords always returns > a true value". It's not a race for greens and percentages. otherwise it should be called "golf"... :) -- Jé

Re: Devel::Cover Problem: testing || for a default value.

2005-07-11 Thread Yuval Kogman
On Mon, Jul 11, 2005 at 19:27:52 +0300, Shlomi Fish wrote: > <<< > my $coords = shift; > > if (!$coords) > { > $coords = $self->get_current_coords(); > } > >>> A work around: $coords = scalar(@_) ? shift : $self->get_current_coords(); > I should also note that I have many > > my

Devel::Cover Problem: testing || for a default value.

2005-07-11 Thread Shlomi Fish
In HTML::Widgets::NavMenu I have the following code: <<< sub get_coords_while_skipping_skips { my $self = shift; my $callback = shift; my $coords = shift || $self->get_current_coords(); # line 571 >>> Now when I run the tests with Devel::Cover it reports the following for line 571 i