robkinyon / dbm-deep

DBM::Deep Perl module
http://search.cpan.org/dist/DBM-Deep/
11 stars 16 forks source link

Use "defined" in DBM::Deep::Array::PUSH #1

Closed jes closed 11 years ago

jes commented 11 years ago

This way it is possible to push 0's and empty strings.

https://rt.cpan.org/Public/Bug/Display.html?id=85414

jes commented 11 years ago

Note this still doesn't fix this code entirely.

Consider:

push @a, undef, "hello";

With normal arrays this works fine, but here it will break the loop before pushing "hello". I'm not sure what to do about this.

jes commented 11 years ago

Should be sorted now.

robkinyon commented 11 years ago

Jes -

What do you think about switching the code to something like:

while ( length @_ ) {
    $self->STORE( $length, shift(@_) );
    $length++;
}

That way, we don't care what the thing is at all. We just care that a thing was passed in, whatever it is.

If you think that works, please amend the patch and (most importantly!) add tests. All the test cases you've suggested are good ideas. :)

And, finally, please add yourself to the CONTRIBUTORS list in the DBM/Deep.pod file.

jes commented 11 years ago

That won't work:

$ perl -e '@a = (); print length(@a), "\n"'
1

The length will never get to 0. For now I'll just add the test and CONTRIBUTORS line. Feel free to change the loop to

while ( @_ ) {
    $self->STORE( $length, shift(@_) );
    $length++;
}

Personally, I think the for loop version is nicer.

jes commented 11 years ago

I think this is suitable for merging now, unless there is anything else you would like me to change?

jes commented 11 years ago

Anything I could do to help?

robkinyon commented 11 years ago

Merged and released. Thanks!

On Tue, Jun 4, 2013 at 9:05 AM, James Stanley notifications@github.comwrote:

Anything I could do to help?

— Reply to this email directly or view it on GitHubhttps://github.com/robkinyon/dbm-deep/pull/1#issuecomment-18919630 .

Thanks, Rob Kinyon

jes commented 11 years ago

Cheers Rob :)

cpansprout commented 11 years ago

Thank you, Rob, for taking care of this and other issues. I just have not had time lately to do anything with DBM::Deep.

robkinyon commented 11 years ago

No worries. That's why there's two of us.

On Fri, Jul 19, 2013 at 12:48 PM, cpansprout notifications@github.comwrote:

Thank you, Rob, for taking care of this and other issues. I just have not had time lately to do anything with DBM::Deep.

— Reply to this email directly or view it on GitHubhttps://github.com/robkinyon/dbm-deep/pull/1#issuecomment-21273009 .

Thanks, Rob Kinyon