Skip to content
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

Add toEightFigureString and toTenFigureString #6

Closed
wants to merge 9 commits into from
Closed

Add toEightFigureString and toTenFigureString #6

wants to merge 9 commits into from

Conversation

stevegoddard
Copy link
Contributor

Edited OSRef to add 2 new functions (cloned from toSixFigureRef) that allow 10m (8 figure) resolution and 1m (10 figure) resolution grid references from the easting / northing of OSRef.

Update to OSRef.php to add toEightFigureString and toTenFigureString. Allows for 10m and 1m accuracy grid references to be created from easting northing.
Add toEightFigureString and totenFigureString
@dvdoug
Copy link
Owner

dvdoug commented Jan 17, 2016

Hi

I'm happy (in theory) to take a PR to add these functions, but I'm not a fan of the amount of copy/pasted code this involves - can you turn the existing toSixFigureString into a function that takes a value (6,8,10), and then outputs a string of the relevant length? Then to[Six/Eight/Ten]FigureString could just be wrappers around it.

@stevegoddard
Copy link
Contributor Author

Excellent idea. I'll have a bash at it tomorrow...

Steve

Added new PRIVATE function 'toGridReference($length)' - private as there is no validation on the $length variable. So can only be called by clild functions. Changed 'toSixFigureReference' to be a wrapper to new function. Added 'toTwoFigureReference', 'toFourFigureReference', 'toEightFigureReference' and 'toTenFigureReference'.
Added additional test to check new 2, 4, 8 and 10 figure grid ref string functions.
@stevegoddard
Copy link
Contributor Author

All done @dvdoug - let me know what you think.

Steve

@dvdoug
Copy link
Owner

dvdoug commented Jan 19, 2016

Thanks for the patch! I've merged it in, and tagged a new release that includes it (v2.0.1)

@dvdoug dvdoug closed this Jan 19, 2016
@stevegoddard
Copy link
Contributor Author

Thanks Doug! Thanks for the package. I’ve been using it for a long time - prior to your work to pull it into Git - when it was just on the jstott site.
I was really pleased to see it on Github and so easy to pull it into projects. It’s such a useful package - glad I was able to contribute a tiny bit!

Steve

On 19 Jan 2016, at 22:06, Doug Wright <notifications@github.commailto:notifications@github.com> wrote:

Thanks for the patch! I've merged it in, and tagged a new release that includes it (v2.0.1)


Reply to this email directly or view it on GitHubhttps://github.com//pull/6#issuecomment-173003247.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants