Skip Menu |

This queue is for tickets about the Hash-Flatten CPAN distribution.

Report information
The Basics
Id: 84033
Status: open
Worked: 30 min
Priority: 0/
Queue: Hash-Flatten

People
Owner: Nobody in particular
Requestors: xiong [...] cpan.org (daily)
Cc:
AdminCc:

Bug Information
Severity: Important
Broken in: 1.19
Fixed in: (no value)



Subject: Inconsistent options settings
POD appears to me to suggest that in the following two client-code lines: $o = new Hash::Flatten(\%options); $flat = $o->flatten( $nested, \%options ); ... that in either case \%options accepts the same options having the same effects. POD is a little unclear on this point but that's how I interpret it; this kind of parallel syntax is common and reasonable. However I find this works: my $do_hash_flat = Hash::Flatten->new(); $flat = $do_hash_flat->flatten($u, { EscapeSequence => '#', DisableEscapes => 1, }); ... while this does not: my $do_hash_flat = Hash::Flatten->new({ EscapeSequence => '#', DisableEscapes => 1, }); $flat = $do_hash_flat->flatten($u); The latter construct emits: Hash delimiter cannot contain escape sequence at... WORKAROUND: Always supply options at point of invocation of object method, not during object construction. Messy!
Hmm, that WORKAROUND doesn't work with HashDelimiter, which must be passed as an option to new(). And passing options both to new() and to flatten() silently overwrites the earlier hashref. I don't actually see any way of setting both HashDelimiter and DisableEscapes to be effective in any given operation. If you'll forgive me, this is perhaps not so much a fault in code as a fault in testing. I've had a look at your test suite and you're not exercising all combinations of options and invocations. Suggest a rewrite of tests to enforce testing of all legitimate combinations, which will lead directly to correction of code; and perhaps a bit of a touch-up of the POD.
The problem with this is in the 'new' subroutine - it does not check that EscapeSequence is not undef before it checks that delimiters include EscapeSequence. This is fixed by replacing the existing 'new' sub with this one: sub new { my ($class, $options) = @_; $options = {} unless ref $options eq 'HASH'; my $self = { %$options }; #Defaults $self->{HashDelimiter} ||= DEFAULT_HASH_DELIM; $self->{ArrayDelimiter} ||= DEFAULT_ARRAY_DELIM; $self->{EscapeSequence} = "\\" unless(defined $self->{EscapeSequence} && length($self->{EscapeSequence}) > 0); $self->{EscapeSequence} = undef if($self->{DisableEscapes}); # Sanity check: shouldn't check against $self->{EscapeSequence} # when $self->{DisableEscapes} is set unless($self->{DisableEscapes}) { #Sanity check: delimiters don't contain escape sequence croak("Hash delimiter cannot contain escape sequence") if( $self->{HashDelimiter} =~ /\Q$self->{EscapeSequence}\E/ ); croak("Array delimiter cannot contain escape sequence") if( $self->{ArrayDelimiter} =~ /\Q$self->{EscapeSequence}\E/ ); } TRACE(__PACKAGE__." constructor - $self"); return bless($self, $class); }