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 methods to direct buffer for putting and getting numbers encoded as ASCII #117

Closed
mjpt777 opened this issue Dec 1, 2017 · 9 comments

Comments

@mjpt777
Copy link
Contributor

mjpt777 commented Dec 1, 2017

Natural numbers
Signed numbers
Decimal numbers

Should be implemented allocation free.

@RichardWarburton
Copy link
Contributor

Artio actually already has methods for this on its AsciiBuffer. I'm happy to PR the additions if you want.

@mjpt777
Copy link
Contributor Author

mjpt777 commented Dec 1, 2017

It would be good to migrate some of them. The implementations from Artio could be better optimised, e.g. single bounds check rather than byte by byte, plus a few other refinements. Artio could then use the versions in Agrona.

@RichardWarburton
Copy link
Contributor

I'm just taking a look at this issue now. What are your thoughts on the correct behaviour for getting a numeric value if the ascii encoded bytes aren't with the range '0' - '9' ? For example I call getInt(4, 3) and the bytes in that range are 'abc'. At the moment I throw an Exception rather than just returning the wrong number but I wonder if that's really the responsibility of the Buffer or of the caller.

@RichardWarburton
Copy link
Contributor

Oh another thought. Do you want the int return type versions and the long return type versions or just the int?

@mjpt777
Copy link
Contributor Author

mjpt777 commented Dec 11, 2017

Thanks @RichardWarburton. I think it should throw a NumberFormatException when not containing 0-9 or -.

@mjpt777
Copy link
Contributor Author

mjpt777 commented Dec 11, 2017

We should offer the parsing of int and long. I think methods should be named parseIntAscii and parseLongAscii to indicate they are performing a parse operation. With the other way being putIntAscii and putLongAscii respectively.

@RichardWarburton
Copy link
Contributor

Cool, thanks for the feedback.

There's also the issue of floats. In Artio we have our own DecimalFloat class that is used for precise encoding of values and can be parsed/written. Is it worth bringing this into Agrona as well, or just using plain Java floats.

@mjpt777
Copy link
Contributor Author

mjpt777 commented Dec 11, 2017

I think for now ints and longs are sufficient and we can decide in time how to handle decimals and floats.

@mjpt777
Copy link
Contributor Author

mjpt777 commented Jan 24, 2018

Will not do floating point for now. Can be raised as a separate issue if required.

@mjpt777 mjpt777 closed this as completed Jan 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants