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

Values of 0 and undef treated the same #23

Closed
pajlpajl opened this Issue Feb 10, 2019 · 6 comments

Comments

Projects
None yet
2 participants
@pajlpajl
Copy link

pajlpajl commented Feb 10, 2019

The method get_shp_shx_header_value incorrectly returns undef instead of 0. Values of 0 are valid for coordinates and occur, say, at the Greenwich meridian. The line below is incorrect:
return $self->{'shx
' . $val} || $self->{'shp_' . $val} || undef;
This line is correct:
return $self->{'shx_' . $val} // $self->{'shp_' . $val} // undef;
The line just a few lines above it
if (!($self->{'shx_' . $val} || $self->{'shp_' . $val})) {
will call $self->_read_shx_header() needlessly a second or more times and would also be improved using defined().

I did not review whether anything similar occurs elsewhere in the code.

@shawnlaffan

This comment has been minimized.

Copy link
Owner

shawnlaffan commented Feb 10, 2019

Thanks for reporting.

As with #22, a pull request with a test would be appreciated.

In terms of the defined-or operator, we currently support 5.8. That said, bumping the minimum perl to 5.10 is an open issue (see #2).

At the very least the trailing || undef could be removed as it is redundant.

@pajlpajl

This comment has been minimized.

Copy link
Author

pajlpajl commented Feb 10, 2019

shawnlaffan added a commit that referenced this issue Feb 10, 2019

shawnlaffan added a commit that referenced this issue Feb 10, 2019

@shawnlaffan

This comment has been minimized.

Copy link
Owner

shawnlaffan commented Feb 10, 2019

[update - fixed incorrect cpan syntax]

A dev release is now on CPAN at https://metacpan.org/release/SLAFFAN/Geo-ShapeFile-2.65_001

Could you please test if this works? If it does, and cpan testers stays green, then I will upload a new version.

You might already know this, but you can test it locally without installing it using these steps (updating path separators as needed for your OS):

cpanm --look Geo::ShapeFile@2.65_001
perl Makefile.PL
gmake
gmake test

perl -Mblib \path\to\your_perl_script_or_app.pl

You can also call it from some other path using

perl -Mblib=\path\to\geo_shapefile_build_dir\blib your_perl_script_or_app.pl

Or you can just add the blib\lib and blib\auto folders to the PERL5LIB env var.

@pajlpajl

This comment has been minimized.

Copy link
Author

pajlpajl commented Feb 11, 2019

shawnlaffan added a commit that referenced this issue Feb 11, 2019

shawnlaffan added a commit that referenced this issue Feb 11, 2019

use &&, not ||
Updates #23
@shawnlaffan

This comment has been minimized.

Copy link
Owner

shawnlaffan commented Feb 11, 2019

@shawnlaffan

This comment has been minimized.

Copy link
Owner

shawnlaffan commented Feb 11, 2019

Marking as fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment