Tests, Documentation
Dear co-hackers, having read quite a bit of Classpath code, I'd like to express two wishes. First, please write more test cases. I agree that it is boring, dumb, and taxing; testing slows down development. But it's also the most reliable way to make sure that code really works. If anyone needs to change our code in five years, they'll want to know whether their change has any serious impact. Without a large test suite, this is close to impossible. Second, please write understandable documentation for every single method. Developers should be able to create free programs without referring to external documents. These could disappear at any time. In other words, I think that we really need to improve the quality of our code. Building a sound foundation for others is not the same as some fun week-end hack. Thank you all, -- Sascha Sascha Brawer, [EMAIL PROTECTED], http://www.dandelis.ch/people/brawer/ ___ Classpath mailing list [EMAIL PROTECTED] http://mail.gnu.org/mailman/listinfo/classpath
Re: new jalopy available
Hi, On Sat, 2003-11-22 at 15:45, Michael Koch wrote: > Serializable classes have to have their fields in the same order as in > in SUNs classes because of serialization issues. I didn't know that. What are the problems precisely? For calculation of the stream unique identifiers (serialVersionUIDs) each field of the class is used sorted by field name. And for an ObjectIn/OutputStream order of the field array shouldn't matter. Cheers, Mark signature.asc Description: This is a digitally signed message part ___ Classpath mailing list [EMAIL PROTECTED] http://mail.gnu.org/mailman/listinfo/classpath
Re: new jalopy available
On Sat, Nov 22, 2003 at 07:57:55AM -0700, Eric Blake wrote: > Michael Koch wrote: > > > >Serializable classes have to have their fields in the same order as in > >in SUNs classes because of serialization issues. > > I thought we already had the style rule that all serialized classes provide > an explicit serialVersionUID - because we cannot depend on the compiler to > generate the same serial version as Sun. Case in point: when a class > literal appears in the class, Sun emits a one-argument class$ method, but > jikes emits a two-argument version; this affects the serial version. Also, > as long as the serial version is correct, deserialization is name-based > (not offset-based). Therefore, field ordering does not matter, so long as > we provide the explicit UID. Thx for clarifying this. It wasnt clear to me. I thought its always offset based. Michael ___ Classpath mailing list [EMAIL PROTECTED] http://mail.gnu.org/mailman/listinfo/classpath
Re: new jalopy available
Michael Koch wrote: Serializable classes have to have their fields in the same order as in in SUNs classes because of serialization issues. I thought we already had the style rule that all serialized classes provide an explicit serialVersionUID - because we cannot depend on the compiler to generate the same serial version as Sun. Case in point: when a class literal appears in the class, Sun emits a one-argument class$ method, but jikes emits a two-argument version; this affects the serial version. Also, as long as the serial version is correct, deserialization is name-based (not offset-based). Therefore, field ordering does not matter, so long as we provide the explicit UID. -- Someday, I might put a cute statement here. Eric Blake [EMAIL PROTECTED] ___ Classpath mailing list [EMAIL PROTECTED] http://mail.gnu.org/mailman/listinfo/classpath
Re: [PATCH] Two methods for NumberFormat
Hi, On Sat, 2003-11-22 at 15:38, Guilhem Lavaux wrote: > >(I would like to see us not use a space between the method and the > >bracket beginning the argument list. But this class already had that and > >jalopy will hopefully catch all this in the future for us.) > > > > > I've just made a probabilistic choice according to what I've seen in > some other files. It seemed to me that gcc-java files tends to have that > sort of practices. If you say it's wrong it doesn't matter I'll fix this. It is not wrong (for now). Just follow what the file already does. Jalopy will fix this for us soon anyway :) Cheers, Mark signature.asc Description: This is a digitally signed message part ___ Classpath mailing list [EMAIL PROTECTED] http://mail.gnu.org/mailman/listinfo/classpath
Re: [PATCH] Compliance of DecimalFormatSymbols
Mark Wielaard wrote: At your service! - ChangeLog entries should start with: date Name email. - Just between filename and (field/method) - Entries are full sentences with starting capital and ending dot. 2003-11-21 Guilhem Lavaux <[EMAIL PROTECTED]> * java/text/DecimalFormatSymbols.java (locale): New field. (DecimalFormatSymbols (Locale)): Set locale. (serialVersionOnStream): Upgraded to number 2. (readObject): Assign locale if it wasn't by the serializer. Guilhem, please feel free to try out your new cvs commit powers with these changes. Thanks, I was needing some sort of real policy for that. I'm now using my new powers to commit these changes. Cheers, Guilhem. ___ Classpath mailing list [EMAIL PROTECTED] http://mail.gnu.org/mailman/listinfo/classpath
Re: new jalopy available
On Sat, Nov 22, 2003 at 07:35:48AM -0700, Eric Blake wrote: > I've noticed two general styles of 'sorting' in Classpath sources. Both > styles separate the metadata types (all fields are listed, followed by > member classes, constructors, methods). Within those groupings, one style > sorts all methods alphabetically, and the other sorts methods according to > the order which they are listed in Sun's documentation (which tends to be, > but isn't always, by common functionality). There is also the question of > whether implementation-only members (private or default access) should be > mixed in with exposed members (protected or public), or whether they come > later. > > I don't have a general opinion - alphabetical sorting makes it easier to > find a method when just reading a file, but following Sun's sorting makes > it easier to compare a method to Sun's documentation of how it should > behave. Either choice is fine with me, especially since emacs has > wrap-around searching. But if Jalopy can make the decision easier, by > enforcing a sorting style, it might be worth considering. Serializable classes have to have their fields in the same order as in in SUNs classes because of serialization issues. Michael ___ Classpath mailing list [EMAIL PROTECTED] http://mail.gnu.org/mailman/listinfo/classpath
Re: [PATCH] Two methods for NumberFormat
Mark Wielaard wrote: Hi Guilhem, On Fri, 2003-11-21 at 21:34, Guilhem Lavaux wrote: I am continuing the series of patches with that one, it adds two methods to NumberFormat: getIntegerInstance() and getIntegerInstance(Locale). They have to be implemented according to Java 1.4. Thanks for splitting this up in small chunks. Three small comments: - You forgot a ChangeLog entry. Well not completely forgotten but I was between two tasks... ;-) - Don't forget to update the copyright year. Ok. - The second comment says 'default locale', should be 'desired locale'. But besides that it looks fine to me. Please check it in. Too many copy-paste... ;-) (I would like to see us not use a space between the method and the bracket beginning the argument list. But this class already had that and jalopy will hopefully catch all this in the future for us.) I've just made a probabilistic choice according to what I've seen in some other files. It seemed to me that gcc-java files tends to have that sort of practices. If you say it's wrong it doesn't matter I'll fix this. Regards, Guilhem. ___ Classpath mailing list [EMAIL PROTECTED] http://mail.gnu.org/mailman/listinfo/classpath
Re: new jalopy available
I've noticed two general styles of 'sorting' in Classpath sources. Both styles separate the metadata types (all fields are listed, followed by member classes, constructors, methods). Within those groupings, one style sorts all methods alphabetically, and the other sorts methods according to the order which they are listed in Sun's documentation (which tends to be, but isn't always, by common functionality). There is also the question of whether implementation-only members (private or default access) should be mixed in with exposed members (protected or public), or whether they come later. I don't have a general opinion - alphabetical sorting makes it easier to find a method when just reading a file, but following Sun's sorting makes it easier to compare a method to Sun's documentation of how it should behave. Either choice is fine with me, especially since emacs has wrap-around searching. But if Jalopy can make the decision easier, by enforcing a sorting style, it might be worth considering. Mark Wielaard wrote: Hi, On Fri, 2003-11-21 at 23:06, Raif S. Naffah wrote: jalopy is able to handle grouping, and sorting, of class elements (e.g.static field and initialisers, instance fields, constructors, etc...) and separating them with a 1-line separator (2 with a blank line followup). how (strongly) do people feel about enabling this? I don't like that very much. I like it when methods are grouped logically (same kind of methods) together. Although simply adding all constructors, static fields and field members together is probably OK. Cheers, Mark -- Someday, I might put a cute statement here. Eric Blake [EMAIL PROTECTED] ___ Classpath mailing list [EMAIL PROTECTED] http://mail.gnu.org/mailman/listinfo/classpath
RE: [PATCH] Fix for Hashtable contains blowing up the stack
Hi, On Sat, 2003-11-22 at 13:06, Jeroen Frijters wrote: > The fix is actually incorrect. containsValue() should call contains() > because containsValue() is a new method (since 1.2) and contains() > exists since 1.0. Older code may have overridden contains() and this > should work with newer code that calls containsValue(). Urgh. Yes, you are right. Wrote an additional test case for this: gnu.testlet.java.util.Hashtable.ContainsHash (it currently fails). > As I've argued before, in cases such as these (multiple virtual methods > that do the same thing), we must use the same delegation as the Sun > implementation to be compatible. Problem is that this is not always easy to know. So we depend on bug reports and then have to write a Mauve test if we think that following such behavior is beneficial. But the "newer calls older" is a guideline that will mostly work (I hope). BTW. People might want the read what Jeroen wrote earlier about it: http://weblog.ikvm.net/commentview.aspx/8b14c003-e16a-4e99-a52e-ad776cbb8cbf His blog entires (and the people who comment on it) are very nice BTW. I am always a bit sad when there is a week without a new entry. Cheers, Mark signature.asc Description: This is a digitally signed message part ___ Classpath mailing list [EMAIL PROTECTED] http://mail.gnu.org/mailman/listinfo/classpath
RE: [PATCH] Fix for Hashtable contains blowing up the stack
Mark Wielaard wrote: > On Fri, 2003-11-21 at 20:48, Dalibor Topic wrote: > > > this is a typical java library bug, and I've both > produced and fixed a > > > couple of those myself ;) You get an infinite recursion > through calling > > > overridden methods. Class A in library implements public > methods b and > > > c, where b internally (and of course undocumentedly ;) > calls c. Class B > > > from some user of the library extends A and overrides c > to call b. When > > > the user calls the method b of class B it goes straight > into an infinite > > > loop. > > Ugh. That is nasty. Especially since when you override > Hashtable it is a natural thing to do since contains and > containsValue are specced to do the same thing. The fix is actually incorrect. containsValue() should call contains() because containsValue() is a new method (since 1.2) and contains() exists since 1.0. Older code may have overridden contains() and this should work with newer code that calls containsValue(). As I've argued before, in cases such as these (multiple virtual methods that do the same thing), we must use the same delegation as the Sun implementation to be compatible. Regards, Jeroen ___ Classpath mailing list [EMAIL PROTECTED] http://mail.gnu.org/mailman/listinfo/classpath
Re: new jalopy available
Hi, On Fri, 2003-11-21 at 23:06, Raif S. Naffah wrote: > jalopy is able to handle grouping, and sorting, of class elements > (e.g.static field and initialisers, instance fields, constructors, > etc...) and separating them with a 1-line separator (2 with a blank > line followup). > > how (strongly) do people feel about enabling this? I don't like that very much. I like it when methods are grouped logically (same kind of methods) together. Although simply adding all constructors, static fields and field members together is probably OK. Cheers, Mark signature.asc Description: This is a digitally signed message part ___ Classpath mailing list [EMAIL PROTECTED] http://mail.gnu.org/mailman/listinfo/classpath
Re: [PATCH] Two methods for NumberFormat
Hi Guilhem, On Fri, 2003-11-21 at 21:34, Guilhem Lavaux wrote: > I am continuing the series of patches with that one, it adds two methods > to NumberFormat: getIntegerInstance() and getIntegerInstance(Locale). > They have to be implemented according to Java 1.4. Thanks for splitting this up in small chunks. Three small comments: - You forgot a ChangeLog entry. - Don't forget to update the copyright year. - The second comment says 'default locale', should be 'desired locale'. But besides that it looks fine to me. Please check it in. (I would like to see us not use a space between the method and the bracket beginning the argument list. But this class already had that and jalopy will hopefully catch all this in the future for us.) Thanks, Mark signature.asc Description: This is a digitally signed message part ___ Classpath mailing list [EMAIL PROTECTED] http://mail.gnu.org/mailman/listinfo/classpath
Re: [PATCH] NumberFormat
Hi,
On Sat, 2003-11-22 at 00:11, Dalibor Topic wrote:
> Looks good to me in general, I have small remarks before you check it in:
Same here. Please check it in. Just one little nitpick.
> > +protected Object readResolve() throws InvalidObjectException
> > +{
> > + String s = getName();
> > + for (int i=0;i > + if (s.equals(allFields[i].getName()))
> > + return allFields[i];
Indenting looks a bit off, but that could be the diff and the
commenting. Please add spaces between the elements of the for loop and
(assignment) operators.
for (int i = 0; i < allFields.length; i++)
> Finally, the mean question: do we have mauve tests for those? If not,
> would you write some, please, so that gcj devs don't go ahead again and
> re-implement everything from scratch as they tend to do ;)? [2]
Your [2] note reference is missing. Don't you like us writing Mauve test
cases for you? (Although I doubt you could create a very good one here,
only the (de)serialization is a little interesting.)
> dalibor topic,
>
> who is eager to write his own patch to the java/text/FieldPosition.java
> (equals) method's comment as soon as he's finished playing virtual mjw ;)
You are getting quite good at it!
> [1] I devote this pun to ricky clarckson. Mark, how are his
> documentation patches coming along?
That was a terrible pun...
Ricky is working through the bureaucrazies, he will not give up, he will
not be defeated, and he will do it when he is ready :)
Cheers,
Mark
signature.asc
Description: This is a digitally signed message part
___
Classpath mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/classpath
Re: [PATCH] Compliance of DecimalFormatSymbols
Hi,
On Sat, 2003-11-22 at 00:39, Dalibor Topic wrote:
> > Here is an adaptation of the former patch concerning java/text.
> > This one is small and concerns only java/text/DecimalFormatSymbols.
> > According to the official specification, if the serial version is greater
> > than 2, you have to read the locale field. This is what is done here.
Thanks. Below are some little (syntactic) nitpicks.
> @@ -580,13 +581,20 @@
>/**
> * @serial This value represents the type of object being de-serialized.
> * 0 indicates a pre-Java 1.1.6 version, 1 indicates 1.1.6 or later.
> - */
> - private int serialVersionOnStream = 1;
> + * 0 indicates a pre-Java 1.1.6 version, 1 indicates 1.1.6 or later,
> + * 2 indicates 1.4 or later
> +*/
> + private int serialVersionOnStream = 2;
Indentation looks strange (hopefully jalopy will catch that for us in the future).
And comments should end with a dot.
>
> + /**
> + * @serial the locale of these currency symbols.
> + */
> + private Locale locale;
Make that comment start with a capital (The local...) so it looks like a real sentence.
> {
> monetarySeparator = decimalSeparator;
> exponential = 'E';
> - serialVersionOnStream = 1;
>}
> +if (serialVersionOnStream < 2)
> + {
> + locale = Locale.getDefault();
> + }
> +serialVersionOnStream = 2;
>}
> }
You don't need brackets when the block is just one line:
if (serialVersionOnStream < 2)
locale = Locale.getDefault();
serialVersionOnStream = 2;
> Looks good to me beside just a small point. Could you write some mauve
> tests to go with it, please?
Already made a very simple one.
gnu/testlet/java/text/DecimalFormatSymbols/serial.java
It can certainly be extended to test for more things.
> > 2003-11-21 Guilhem Lavaux <[EMAIL PROTECTED]>
> >
> > * java/text/DecimalFormatSymbols.java(locale): New field.
> > (serialVersionOnStream): Upgraded to number 2
> > (readObject): updated to assign locale if it wasn't by the
> > serializer.
>
> Lacks one entry:
> (DecimalFormatSymbols (Locale)): Set locale.
>
> Other than that, looks fine to me. I'm waiting for the ChangeLog police,
> though ;)
At your service!
- ChangeLog entries should start with:
date Name email.
- Just between filename and (field/method)
- Entries are full sentences with starting capital and ending dot.
2003-11-21 Guilhem Lavaux <[EMAIL PROTECTED]>
* java/text/DecimalFormatSymbols.java (locale): New field.
(DecimalFormatSymbols (Locale)): Set locale.
(serialVersionOnStream): Upgraded to number 2.
(readObject): Assign locale if it wasn't by the serializer.
Guilhem, please feel free to try out your new cvs commit powers with
these changes.
Cheers,
Mark
signature.asc
Description: This is a digitally signed message part
___
Classpath mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/classpath
