Hi Guy
I just had a glance at the code of UnsignedLong. Just my observations:
- The first thing that popped into my eye was the constructor containing unreachable Code (Line 30 - and therefore unnecessary 29/31).
- Line 36 references ulongValue (the field) which has not been initialized so far. Afaik this is always 0 and throwing an IllegalArgumentException
does neither make sense on this condition, nor will it ever happen.
- I would not assign "value" to ulongValue in castulong (Line 45), as this might contain the negative long.
- Line 47 could be replaced by "castUlongValue.bit_63 = value < 0; Which would be easier and correct. While the current version is not. If value
contains for example Long.MIN_VALUE and it is cast to int, the result will be 0.
- The equals method only compares the ulongValue and ignores the bit_63. I might have the intention of the ulongValue interpreted incorrectly,
but that seems essential for the comparison as well. As a general rule, if you implement the equals method, you should also implement hashCode.
- The implementation of compareTo is in my opinion a bit complex. The condition "bit_63 != compareValue.bit_63" would be much more obvious. As
ulongValue could be inconsistent (either negative from the cast or positive from the constructor taking value and msb) the comparison is not always correct.
- I would call the methods converting UnsignedLong to int, long, float and double casts, since they do not necessarily result in the represented
value.
- I did not check the correctness of the arithmetic operations, but do you have any unit tests for them?
If I'm correct the benefit of this class could also be achieved with a simple wrapper for a long, which does not require the bit_63 boolean, as
it is already available in the 63th bit of the ulongValue anyway. Currently, the implementation does not provide the class invariant I would expect. Beyond I think the major problem with the approach is, that there is no guarantee that the values we need to
represent are only 64bit - it could be more.
Thanks for your effort, but in my opinion in the current shape it is hardly possible to use this code in CDT - sorry.
Regards
Thomas
From: cdt-dev-bounces@xxxxxxxxxxx [mailto:cdt-dev-bounces@xxxxxxxxxxx]
On Behalf Of Guy.Bonneau@xxxxxxxxxxx
Sent: Dienstag, 9. April 2013 19:43
To: CDT General developers list.
Cc: cdt-dev-bounces@xxxxxxxxxxx
Subject: Re: [cdt-dev] Bug ID 403404
The included class file should be very close to what is needed. It is not thoroughly tested however. I did not try to fully optimize the computing but the class should provide the functionality
that is needed. It is not well commented also. May be someone can review it and test it .If something is missing or the testing fail let me know.
Regards.
Guy(See attached file: UnsignedLong.java)
Doug
Schaefer ---04/05/2013 09:37:09 AM---I think you need to look at the implementation of BigInteger before claiming it's not a performance
From:
Doug Schaefer <dschaefer@xxxxxxx>
To:
CDT General developers list. <cdt-dev@xxxxxxxxxxx>,
David Wang <kuoweiwang@xxxxxxxxx>,
Sergey Prigogin <eclipse.sprigogin@xxxxxxxxx>,
Date:
04/05/2013 09:37 AM
Subject:
Re: [cdt-dev] Bug ID 403404
Sent by:
cdt-dev-bounces@xxxxxxxxxxx
I think you need to look at the implementation of BigInteger before claiming it's not a performance problem. At the very least, it's really big.
It's easy to imagine a much more performant and small implementation if you know how many bits you're trying to handle. For example, with 128-bits, you can use double word algorithms are as old
as I am and maximize performance.
Doug.
From:
<Oberhuber>, Martin <Martin.Oberhuber@xxxxxxxxxxxxx>
Reply-To: "CDT General developers list." <cdt-dev@xxxxxxxxxxx>
Date: Friday, 5 April, 2013 8:51 AM
To: David Wang <kuoweiwang@xxxxxxxxx>, "CDT General developers list."
<cdt-dev@xxxxxxxxxxx>, Sergey Prigogin <eclipse.sprigogin@xxxxxxxxx>
Subject: Re: [cdt-dev] Bug ID 403404
StackOverflow recommends BigInteger and doesn’t talk about inefficiency.
Starting with BigInteger now, it should be fairly straightforward to refactor into a different class with the same API later (ie simple search & replace on the name BigInteger and
the import statements for its package). That is, only if performance measurement shows that it’s in fact a problem.
Or are you concerned that the API of BigInteger is already inefficient ? – Are you concerned about memory size or performance ?
Thanks,
Martin
--
Martin Oberhuber, SMTS / Product Architect – Development Tools,
Wind River
direct +43.662.457915.85 fax +43.662.457915.6
From: cdt-dev-bounces@xxxxxxxxxxx [mailto:cdt-dev-bounces@xxxxxxxxxxx]
On Behalf Of David Wang
Sent: Wednesday, April 03, 2013 1:39 AM
To: Sergey Prigogin
Cc: CDT General developers list.
Subject: Re: [cdt-dev] Bug ID 403404
I would hope we can simply use the BigInteger for now even though its inefficient. But at least this stuff would
work. At the same time we can look into a more solid solution.
Thanks
From: Sergey Prigogin <eclipse.sprigogin@xxxxxxxxx>
To: David Wang <kuoweiwang@xxxxxxxxx>
Cc: CDT General developers list. <cdt-dev@xxxxxxxxxxx>
Sent: Tuesday, April 2, 2013 4:18 PM
Subject: Re: [cdt-dev] Bug ID 403404
Good opportunity for somebody to contribute one.
-sergey
On Tue, Apr 2, 2013 at 4:15 PM, David Wang <kuoweiwang@xxxxxxxxx> wrote:
Not that I am aware of.
From: Sergey Prigogin <eclipse.sprigogin@xxxxxxxxx>
To: David Wang <kuoweiwang@xxxxxxxxx>
Cc: CDT General developers list. <cdt-dev@xxxxxxxxxxx>
Sent: Tuesday, April 2, 2013 4:12 PM
Subject: Re: [cdt-dev] Bug ID 403404
Oh, I see. I didn't notice the bug number in the subject. Do you know of an open source Java implementation of 128-bit integers with EPL-compatible license? I'd like to avoid BigInteger since it's too heavy and inefficient.
-sergey
On Tue, Apr 2, 2013 at 4:02 PM, David Wang <kuoweiwang@xxxxxxxxx> wrote:
Thanks. I already filed a bug and the ID on the subject. Do you want me to create another one?
I wonder what is the outlook for the bug, my code needs this thing to work.
Thanks again
From: Sergey Prigogin <eclipse.sprigogin@xxxxxxxxx>
To: David Wang <kuoweiwang@xxxxxxxxx>; CDT General developers list. <cdt-dev@xxxxxxxxxxx>
Sent: Tuesday, April 2, 2013 3:52 PM
Subject: Re: [cdt-dev] Bug ID 403404
This is most likely caused by the fact that ULONG_MAX cannot be represented by a Java long. Could you please file a bug.
-sergey
On Tue, Apr 2, 2013 at 3:42 PM, David Wang <kuoweiwang@xxxxxxxxx> wrote:
Hi
I discussed some potential eclipse indexing error at
http://www.eclipse.org/forums/index.php/m/1022322/#msg_1022322
It seems to be a bug in eclipse. I created a bug last month. Can someone please help and confirm
if this is an eclipse bug? and/or will it be fixed shortly?
Thanks a lot, my code heavily relies on this to be working correctly...
David
_______________________________________________
cdt-dev mailing list
cdt-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/cdt-dev
_______________________________________________
cdt-dev mailing list
cdt-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/cdt-dev
DISCLAIMER: Privileged and/or Confidential information may be contained in this message. If you are not the addressee of this message, you may not copy, use or deliver this message to anyone. In such event, you should destroy the message and kindly notify
the sender by reply e-mail. It is understood that opinions or conclusions that do not relate to the official business of the company are neither given nor endorsed by the company. Thank You.
