andrewbissada / kryo

Automatically exported from code.google.com/p/kryo
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Change type (int->long) of field total in class Output return int #142

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
int output.total()

What is the expected output? What do you see instead?
long output.total()

What version of the Kryo are you using?
2.22

Original issue reported on code.google.com by khmelevs...@gmail.com on 18 Oct 2013 at 9:33

GoogleCodeExporter commented 9 years ago
Can you elaborate a bit? Why do you want it to be changed to long? Is is the 
only method whose signature should be changed or do you see more of those?

Original comment by romixlev on 19 Oct 2013 at 7:18

GoogleCodeExporter commented 9 years ago
I implemented the save data from the database to a file. When saving, I show 
the number of recorded objects and the size of the serialized data.
Now to calculate the size I have to consider Integer.MAX_VALUE, because more 
than 2GB in size, the value is negative.

Original comment by khmelevs...@gmail.com on 22 Oct 2013 at 10:49

GoogleCodeExporter commented 9 years ago
OK. I understand your point now. 

But if you look at different JDK stream classes (DataOutputStream, InputStream, 
etc), they all use "int" for returning the number of available bytes and so on. 
And they all wrap around Integer.MAX_VALUE.  So, we are simply following the 
JDK way of doing things at the moment. And I'm not sure that it makes a lot of 
sense to make Kryo IO streams different from other streams...

Original comment by romixlev on 22 Oct 2013 at 12:03

GoogleCodeExporter commented 9 years ago
In InputStream int is used for quantities that don't exceed the 2GB limit.

DataOutputStream.size() returning int is a VERY bad design decision in my 
opinion.
I'd bet that most of the people using that API don't even realize that it can 
overflow.

I'd suggest not copying bad design decisions from the JDK.

Original comment by R.Ger...@gmail.com on 22 Oct 2013 at 12:22

GoogleCodeExporter commented 9 years ago
Changing Input/Output#total to long is probably fine for Kryo.

Original comment by nathan.s...@gmail.com on 22 Oct 2013 at 1:05

GoogleCodeExporter commented 9 years ago
This issue was closed by revision r423.

Original comment by romixlev on 22 Oct 2013 at 8:25

GoogleCodeExporter commented 9 years ago
Fixed in trunk (r423).

Original comment by romixlev on 22 Oct 2013 at 8:27