-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use "defined" in DBM::Deep::Array::PUSH #1
Conversation
This way it is possible to push 0's and empty strings.
Note this still doesn't fix this code entirely. Consider:
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. |
The previous commit would exit the loop upon an undef element, whereas this version always iterates over all of the arguments.
Should be sorted now. |
Jes - What do you think about switching the code to something like:
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. |
That won't work:
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
Personally, I think the for loop version is nicer. |
I think this is suitable for merging now, unless there is anything else you would like me to change? |
Anything I could do to help? |
Use "defined" in DBM::Deep::Array::PUSH
Merged and released. Thanks! On Tue, Jun 4, 2013 at 9:05 AM, James Stanley [email protected]:
Thanks, |
Cheers Rob :) |
Thank you, Rob, for taking care of this and other issues. I just have not had time lately to do anything with DBM::Deep. |
No worries. That's why there's two of us. On Fri, Jul 19, 2013 at 12:48 PM, cpansprout [email protected]:
Thanks, |
This way it is possible to push 0's and empty strings.
https://rt.cpan.org/Public/Bug/Display.html?id=85414