Skip Menu |

Preferred bug tracker

Please visit the preferred bug tracker to report your issue.

This queue is for tickets about the Perl-Critic CPAN distribution.

Report information
The Basics
Id: 82966
Status: open
Priority: 0/
Queue: Perl-Critic
People
Owner: Nobody in particular
Requestors: victor [...] vsespb.ru
Cc:
AdminCc:
Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)

History Show all quoted text
  Fri Jan 25 18:37:46 2013 victor [...] vsespb.ru - Ticket created
Subject: Use the Readonly or Const::Fast module when comparing with Some::Module::VERSION
1.4 is not one of the allowed literal values (0, 1, 2). Use the Readonly or Const::Fast module or the "constant" pragma in the line: my $meta_coder = ($JSON::XS::VERSION >= 1.4) ? JSON::XS->new->utf8->max_depth(1)->max_size(MAX_SIZE) : # some additional abuse-protection JSON::XS->new->utf8; # it's still protected by length checking below (i.e. when I have to work around different behavious of older version of modules). I think maybe it's possible to detect $Some::Module::VERSION in the comparsion.
  Tue Jan 29 18:56:09 2013 jeff [...] imaginative-software.com - Correspondence added
Subject: Re: [rt.cpan.org #82966] Use the Readonly or Const::Fast module when comparing with Some::Module::VERSION
Date: Tue, 29 Jan 2013 11:36:45 -0800
To: bug-Perl-Critic [...] rt.cpan.org
From: Jeffrey Ryan Thalhammer <jeff [...] imaginative-software.com>
On Jan 25, 2013, at 3:37 PM, Victor Efimov via RT wrote: Show quoted text
> I think maybe it's possible to detect $Some::Module::VERSION in the > comparsion.
I think you're asking for this policy to overlook magic numbers in the context of $VERSION variables. I can understand how this might seem less ambiguous than other contexts, but I still think the code would benefit from using a named variable rather than a magic number. For example: use constant FIRST_JSON_XS_VERSION_WITH_LENGTH_PROTECTION => 1.4; my $meta_coder = ($JSON::XS::VERSION >= FIRST_JSON_XS_VERSION_WITH_LENGTH_PROTECTION) ? JSON::XS->new->utf8->max_depth(1)->max_size(MAX_SIZE) # some additional abuse-protection : JSON::XS->new->utf8; # it's still protected by length checking below In this particular case, I agree that looks a bit wordy. But in general, named variables are still a good idea, even for comparing to $VERSION. Give it a try and see how it feels :) From looking at the Policy documentation, it seems that other folks (at least Elliot) have also considered special handling for $VERSION comparisons. So I'm going to leave this ticket open for now. -Jeff
  Tue Jan 29 18:56:11 2013 The RT System itself - Status changed from 'new' to 'open'
  Tue Jan 29 19:26:29 2013 victor [...] vsespb.ru - Correspondence added
Ok, thanks. I'll think about it - maybe I'll try. As you leave it open - I want to make a remark that there is perl UNIVERSAL package http://perldoc.perl.org/UNIVERSAL.html with VERSION() function. I.e. one can use JSON::XS->VERSION() JSON::XS->VERSION(1.4) JSON::XS->VERSION $JSON::XS::VERSION On Wed Jan 30 03:56:09 2013, jeff@imaginative-software.com wrote: Show quoted text
> > On Jan 25, 2013, at 3:37 PM, Victor Efimov via RT wrote: >
> > I think maybe it's possible to detect $Some::Module::VERSION in the > > comparsion.
> > I think you're asking for this policy to overlook magic numbers in the > context of $VERSION variables. I can understand how this might > seem less ambiguous than other contexts, but I still think the code > would benefit from using a named variable rather than a magic > number. For example: > > use constant FIRST_JSON_XS_VERSION_WITH_LENGTH_PROTECTION => 1.4; > > my $meta_coder = ($JSON::XS::VERSION >= > FIRST_JSON_XS_VERSION_WITH_LENGTH_PROTECTION) > ? JSON::XS->new->utf8->max_depth(1)->max_size(MAX_SIZE) # some > additional abuse-protection > : JSON::XS->new->utf8; # it's still protected by length checking > below > > In this particular case, I agree that looks a bit wordy. But in > general, named variables are still a good idea, even for comparing > to $VERSION. Give it a try and see how it feels :) > > From looking at the Policy documentation, it seems that other folks > (at least Elliot) have also considered special handling for > $VERSION comparisons. So I'm going to leave this ticket open for > now. > > -Jeff