[common] code review

[Dain Sundstrom <da...@coredevelopers.net>]

[These are just my personal comments, and notes gathered while reading 
the code]

The nested exception classes should be removed.  Exception nesting is 
handled quite well by the Java 1.4 platform classes.

We don't need an UnreachableStatementException anymore.  Instead you 
should throw an AssertionError (this is noted in the assertion 
facilitates documentation).

I'm not sure why we need the platform.Java class at all right now, as 
we only support 1.4, and our stuff won't even run on an earlier 
version.  If we do keep it, it should use a type safe enumeration.  I 
am happy to make this change.

-dain

/*************************
  * Dain Sundstrom
  * Partner
  * Core Developers Network
  *************************/

Re: [common] code review

[Jason Dillon <ja...@coredevelopers.net>]

> Is there a reason why the jakarta commons lang package was not used 
> for this kind of common jobs ?
> o.a.c.l.SystemUtils give you plateform and Java version
> There are also some NestedException stuffs (Even if using Java 1.4 
> platform seems to be a better idea)

I will have a look at SystemUtils to see if it does the job.  I know 
there are some nestable exceptions there too, but did not have time to 
play with them, so I used my versions for the time being.

--jason

Re: [common] code review

[Emmanuel Bernard <ep...@yahoo.fr>]

Jason Dillon wrote:

>> I'm not sure why we need the platform.Java class at all right now, as 
>> we only support 1.4, and our stuff won't even run on an earlier 
>> version.  If we do keep it, it should use a type safe enumeration.  I 
>> am happy to make this change.
>
>
> I would keep it, so that we can use it to detect the VM version 
> easily.  If you want to turn it into a type safe enum go for it.
>
Is there a reason why the jakarta commons lang package was not used for 
this kind of common jobs ?
o.a.c.l.SystemUtils give you plateform and Java version
There are also some NestedException stuffs (Even if using Java 1.4 
platform seems to be a better idea)

Emmanuel

Re: [common] code review

[Jason Dillon <ja...@coredevelopers.net>]

> The nested exception classes should be removed.  Exception nesting is 
> handled quite well by the Java 1.4 platform classes.

Fine w/me as long as the stack traces for the native throwables a 
readable with deep nesting.  Meant to look into this, but did not.


> We don't need an UnreachableStatementException anymore.  Instead you 
> should throw an AssertionError (this is noted in the assertion 
> facilitates documentation).

Perhaps should be changed to UnreachableStatementError which extends 
AssertionError so that USE could handle the details of message 
generation.


> I'm not sure why we need the platform.Java class at all right now, as 
> we only support 1.4, and our stuff won't even run on an earlier 
> version.  If we do keep it, it should use a type safe enumeration.  I 
> am happy to make this change.

I would keep it, so that we can use it to detect the VM version easily. 
  If you want to turn it into a type safe enum go for it.

--jason