search

jenetics / jpx

JPX - Java GPX library
Apache License 2.0
202 stars 30 forks source link

Issues/jpx 72 parse location #139

Closed bunkenburg closed 3 years ago

bunkenburg commented 3 years ago

Fixes https://github.com/jenetics/jpx/issues/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 commented 3 years ago

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 commented 3 years ago

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

jenetics commented 3 years ago

@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.

bunkenburg commented 3 years ago

Yes, LocationFormatter.parse is threadsafe. The only state in LocationFormatter is _formats which is not changed after creation of an instance of LocationFormatter.

On Mon, 18 Jan 2021 at 18:17, Franz Wilhelmstötter notifications@github.com wrote:

@bunkenburg https://github.com/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.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jenetics/jpx/pull/139#issuecomment-762378623, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGE7TNBM4QPKFZW576JI33S2RUKVANCNFSM4UYV5J4Q .

-- Dr Alexander Bunkenburg www.inspiracio.cat N 41°22.820' E2°08.150' 51m