Skip to content

Issues/jpx 72 parse location#139

Merged
jenetics merged 7 commits intojenetics:r2.2.0from
bunkenburg:issues/JPX-72-parse-location
Jan 22, 2021
Merged

Issues/jpx 72 parse location#139
jenetics merged 7 commits intojenetics:r2.2.0from
bunkenburg:issues/JPX-72-parse-location

Conversation

@bunkenburg
Copy link
Copy Markdown
Contributor

Fixes #72

I've tightened the formatting and fixed one or two small bugs in it.
I added LocationFormatter.parse method and some tests for it.

@jenetics
Copy link
Copy Markdown
Owner

Many thanks, @bunkenburg. I will have a look at it. I'm not sure if I will have time before X-Mas, but I try to review it this year.

@jenetics
Copy link
Copy Markdown
Owner

Sorry for the delay, @bunkenburg, but I needed the timeout. I will soon start the review :-)

@jenetics
Copy link
Copy Markdown
Owner

@bunkenburg thank you for the implementation. I just had a look at the code, finally ;-) It looks good. I only have one question: Is the LocationFormatter.parse method thread safe? I'm not sure, because the used Field class is mutable (Field.setPrefixSign). I would prefer to have this class immutable, if possible.

If the parsing method is thread safe I would merge the PR into the r.2.2.0 branch and leaf the refactoring to an immutable Field class for a later improvement.

@jenetics jenetics changed the base branch from master to r2.2.0 January 22, 2021 20:20
@jenetics jenetics added this to the v2.2.0 milestone Jan 22, 2021
@jenetics jenetics merged commit 8c94271 into jenetics:r2.2.0 Jan 22, 2021
@bunkenburg
Copy link
Copy Markdown
Contributor Author

bunkenburg commented Jan 22, 2021 via email

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Parsing of ISO 6709 location strings

2 participants