Proposed guidelines for SIS development

[Martin Desruisseaux <ma...@geomatys.fr>]

Hello Marc and all

Thanks Marc for the commits on the shapefile module. Can I take this
opportunity for checking if we agree on some guidelines for SIS
developments?

Do not forget to use "svn move" when moving files (do not rely too much
of the IDE for doing the proper thing), otherwise history is lost. For
example AbstractResultSet, AbstractCollection, etc. lost their history
as a result of their move.

The new committed classes put the "abstract" or "final" keywords before
the "public", "protected" or "private" ones, while the rest of Apache
SIS uses the reverse order. While it is technically not required, I
would like a consistent coding convention for Apache SIS. The order used
by the rest of Apache SIS is consistent with the customary order as
specified by the section 8.1.1 of the Java Language Specification [1],
which is also the order used by the JDK source code and tools like
"javap". Do we agree to follow this customary order?

I suggest to never use java.util.Logger.getLogger(String) for getting a
logger, but rather org.apache.sis.logging.Logging.getLogger. It is
almost the same, except that the later gives us a possibility to
redirect to Apache Common loggins or Log4J if desired. This is really
needed when using Apache SIS in some larger applications which require
the use of Log4J (using any other framework causes all our logging
messages to disappear - I agree it is silly, but such applications exist).

The new AbstractAutoChecker and AbstractAutoCheckerList classes in the
sis-utility module duplicate the functionalities of
IndexedResourceBundle. While I did no benchmark, I would expect
IndexedResourceBundle to have higher performance mostly because it
caches the MessageFormat (it also loads the resources from a binary file
and uses index for accessing directly the resources, but the effect is
probably much smaller). It also provides compile-time safety for the
keys, integrates with the internationalization mechanism of the Java
logging framework (which can defer MessageFormat usage until needed) and
with the InternationalString defined by GeoAPI. I would suggest to use
IndexedResourceBundle as a consistent localization mechanism for Apache
SIS. What do you think?

    Martin


[1] http://docs.oracle.com/javase/specs/jls/se7/html/jls-8.html#jls-8.1.1

Re: Proposed guidelines for SIS development

["Mattmann, Chris A (3980)" <ch...@jpl.nasa.gov>]

Hey Martin,

+1 these all make sense to me.

Cheers,
Chris


++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Chris Mattmann, Ph.D.
Chief Architect
Instrument Software and Science Data Systems Section (398)
NASA Jet Propulsion Laboratory Pasadena, CA 91109 USA
Office: 168-519, Mailstop: 168-527
Email: chris.a.mattmann@nasa.gov
WWW:  http://sunset.usc.edu/~mattmann/
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Adjunct Associate Professor, Computer Science Department
University of Southern California, Los Angeles, CA 90089 USA
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++






-----Original Message-----
From: Martin Desruisseaux <ma...@geomatys.fr>
Organization: Geomatys
Reply-To: "dev@sis.apache.org" <de...@sis.apache.org>
Date: Wednesday, December 10, 2014 at 12:00 AM
To: Apache SIS <de...@sis.apache.org>
Subject: Proposed guidelines for SIS development

>Hello Marc and all
>
>Thanks Marc for the commits on the shapefile module. Can I take this
>opportunity for checking if we agree on some guidelines for SIS
>developments?
>
>Do not forget to use "svn move" when moving files (do not rely too much
>of the IDE for doing the proper thing), otherwise history is lost. For
>example AbstractResultSet, AbstractCollection, etc. lost their history
>as a result of their move.
>
>The new committed classes put the "abstract" or "final" keywords before
>the "public", "protected" or "private" ones, while the rest of Apache
>SIS uses the reverse order. While it is technically not required, I
>would like a consistent coding convention for Apache SIS. The order used
>by the rest of Apache SIS is consistent with the customary order as
>specified by the section 8.1.1 of the Java Language Specification [1],
>which is also the order used by the JDK source code and tools like
>"javap". Do we agree to follow this customary order?
>
>I suggest to never use java.util.Logger.getLogger(String) for getting a
>logger, but rather org.apache.sis.logging.Logging.getLogger. It is
>almost the same, except that the later gives us a possibility to
>redirect to Apache Common loggins or Log4J if desired. This is really
>needed when using Apache SIS in some larger applications which require
>the use of Log4J (using any other framework causes all our logging
>messages to disappear - I agree it is silly, but such applications exist).
>
>The new AbstractAutoChecker and AbstractAutoCheckerList classes in the
>sis-utility module duplicate the functionalities of
>IndexedResourceBundle. While I did no benchmark, I would expect
>IndexedResourceBundle to have higher performance mostly because it
>caches the MessageFormat (it also loads the resources from a binary file
>and uses index for accessing directly the resources, but the effect is
>probably much smaller). It also provides compile-time safety for the
>keys, integrates with the internationalization mechanism of the Java
>logging framework (which can defer MessageFormat usage until needed) and
>with the InternationalString defined by GeoAPI. I would suggest to use
>IndexedResourceBundle as a consistent localization mechanism for Apache
>SIS. What do you think?
>
>    Martin
>
>
>[1] http://docs.oracle.com/javase/specs/jls/se7/html/jls-8.html#jls-8.1.1
>

Re: Proposed guidelines for SIS development

[Charitha Madusankha <ch...@gmail.com>]

+1
On Dec 14, 2014 7:01 PM, "Adam Estrada" <es...@gmail.com> wrote:

> +1
>
>
>
> > On Dec 14, 2014, at 4:53 AM, Martin Desruisseaux <
> martin.desruisseaux@geomatys.fr> wrote:
> >
> > Hello all
> >
> > Thanks Marc for the feedback.
> >
> > About AbstractAutoChecker, no problem as the goal sound nice. I suggest
> > to just move them in some internal package until the API settle down. We
> > can move it back to a public package (it can be logging, or other) once
> > the API look fine.
> >
> >    Martin
> >
> >
> > Le 14/12/14 18:47, Marc Le Bihan a écrit :
> >> => The goals of these (yet still incomplete) classes are not to log
> >> messages, even if they are doing this too, it's to grant the objects
> >> inheriting them the ability of auto-checking themselves, and if they
> >> find some troubles then, to list all the errors, warnings they can
> >> find and not only the first one. But, I have not put the parts doing
> >> the auto-checking of the objets yet and I understand really well why
> >> find these classes gruesome ! I am really sorry ! Soon I will complete
> >> these classes.
> >> The overall idea of these classes is to allow the API to check an
> >> object entirely before doing something important with it. Most of the
> >> time, I use this feature before inserting or updating a business
> >> object into a database. I ask, for example, my Customer object about
> >> itself : "Do you feel ok ?", if the object replies : "no, I have no
> >> Name" or something else, I will throw an InvalidCustomerException.
> >> Because if I carry a Customer object from function to function it has
> >> to be ok (in memory at least) : it has respect some core rules. This
> >> kind of defensive programming is really useful when you are reading
> >> business objects from a database that has not be written by you and
> >> seldom contains errors you cannot figure easily : you detect theses
> >> errors immediately. And silently or not, depending what you want.
> >> I will move this classes outside the log package, even if they will
> >> stay on utility jar.
> >>
> >> Marc.
> >
>

Re: Proposed guidelines for SIS development

[Adam Estrada <es...@gmail.com>]

+1 



> On Dec 14, 2014, at 4:53 AM, Martin Desruisseaux <ma...@geomatys.fr> wrote:
> 
> Hello all
> 
> Thanks Marc for the feedback.
> 
> About AbstractAutoChecker, no problem as the goal sound nice. I suggest
> to just move them in some internal package until the API settle down. We
> can move it back to a public package (it can be logging, or other) once
> the API look fine.
> 
>    Martin
> 
> 
> Le 14/12/14 18:47, Marc Le Bihan a écrit :
>> => The goals of these (yet still incomplete) classes are not to log
>> messages, even if they are doing this too, it's to grant the objects
>> inheriting them the ability of auto-checking themselves, and if they
>> find some troubles then, to list all the errors, warnings they can
>> find and not only the first one. But, I have not put the parts doing
>> the auto-checking of the objets yet and I understand really well why
>> find these classes gruesome ! I am really sorry ! Soon I will complete
>> these classes.
>> The overall idea of these classes is to allow the API to check an
>> object entirely before doing something important with it. Most of the
>> time, I use this feature before inserting or updating a business
>> object into a database. I ask, for example, my Customer object about
>> itself : "Do you feel ok ?", if the object replies : "no, I have no
>> Name" or something else, I will throw an InvalidCustomerException.
>> Because if I carry a Customer object from function to function it has
>> to be ok (in memory at least) : it has respect some core rules. This
>> kind of defensive programming is really useful when you are reading
>> business objects from a database that has not be written by you and
>> seldom contains errors you cannot figure easily : you detect theses
>> errors immediately. And silently or not, depending what you want.
>> I will move this classes outside the log package, even if they will
>> stay on utility jar.
>> 
>> Marc.
> 

Re: Proposed guidelines for SIS development

[Martin Desruisseaux <ma...@geomatys.fr>]

Hello all

Thanks Marc for the feedback.

About AbstractAutoChecker, no problem as the goal sound nice. I suggest
to just move them in some internal package until the API settle down. We
can move it back to a public package (it can be logging, or other) once
the API look fine.

    Martin


Le 14/12/14 18:47, Marc Le Bihan a écrit :
> => The goals of these (yet still incomplete) classes are not to log
> messages, even if they are doing this too, it's to grant the objects
> inheriting them the ability of auto-checking themselves, and if they
> find some troubles then, to list all the errors, warnings they can
> find and not only the first one. But, I have not put the parts doing
> the auto-checking of the objets yet and I understand really well why
> find these classes gruesome ! I am really sorry ! Soon I will complete
> these classes.
> The overall idea of these classes is to allow the API to check an
> object entirely before doing something important with it. Most of the
> time, I use this feature before inserting or updating a business
> object into a database. I ask, for example, my Customer object about
> itself : "Do you feel ok ?", if the object replies : "no, I have no
> Name" or something else, I will throw an InvalidCustomerException.
> Because if I carry a Customer object from function to function it has
> to be ok (in memory at least) : it has respect some core rules. This
> kind of defensive programming is really useful when you are reading
> business objects from a database that has not be written by you and
> seldom contains errors you cannot figure easily : you detect theses
> errors immediately. And silently or not, depending what you want.
> I will move this classes outside the log package, even if they will
> stay on utility jar.
>
> Marc.

Re: Proposed guidelines for SIS development

[Martin Desruisseaux <ma...@geomatys.fr>]

Le 14/12/14 18:47, Marc Le Bihan a écrit :
> => I specified a move in Tortoise SVN I was using, I check what happened.

I suggest to run "svn status" from the command-line before to commit. It
the operation is recorded as a move, then "svn status" should display
that fact with information like "file moved to XYZ".

    Martin

Re: Proposed guidelines for SIS development

[Marc Le Bihan <ml...@gmail.com>]

Hello  !

Do not forget to use "svn move" when moving files (do not rely too much
of the IDE for doing the proper thing), otherwise history is lost. For
example AbstractResultSet, AbstractCollection, etc. lost their history
as a result of their move.
=> I specified a move in Tortoise SVN I was using, I check what happened.

The new committed classes put the "abstract" or "final" keywords before
the "public", "protected" or "private" ones, while the rest of Apache
SIS uses the reverse order. While it is technically not required, I
would like a consistent coding convention for Apache SIS. The order used
by the rest of Apache SIS is consistent with the customary order as
specified by the section 8.1.1 of the Java Language Specification [1],
which is also the order used by the JDK source code and tools like
"javap". Do we agree to follow this customary order?
=> OK.

I suggest to never use java.util.Logger.getLogger(String) for getting a
logger, but rather org.apache.sis.logging.Logging.getLogger. It is
almost the same, except that the later gives us a possibility to
redirect to Apache Common loggins or Log4J if desired. This is really
needed when using Apache SIS in some larger applications which require
the use of Log4J (using any other framework causes all our logging
messages to disappear - I agree it is silly, but such applications exist).
=> org.apache.sis.logging.Logging was carrying others dependencies from the 
project and I wished to have a driver for Squirrel made of one or two jars 
only.
But I correct this.

The new AbstractAutoChecker and AbstractAutoCheckerList classes in the
sis-utility module duplicate the functionalities of
IndexedResourceBundle. While I did no benchmark, I would expect
IndexedResourceBundle to have higher performance mostly because it
caches the MessageFormat (it also loads the resources from a binary file
and uses index for accessing directly the resources, but the effect is
probably much smaller). It also provides compile-time safety for the
keys, integrates with the internationalization mechanism of the Java
logging framework (which can defer MessageFormat usage until needed) and
with the InternationalString defined by GeoAPI. I would suggest to use
IndexedResourceBundle as a consistent localization mechanism for Apache
SIS. What do you think?
=> The goals of these (yet still incomplete) classes are not to log 
messages, even if they are doing this too, it's to grant the objects 
inheriting them the ability of auto-checking themselves, and if they find 
some troubles then, to list all the errors, warnings they can find and not 
only the first one. But, I have not put the parts doing the auto-checking of 
the objets yet and I understand really well why find these classes gruesome 
! I am really sorry ! Soon I will complete these classes.
The overall idea of these classes is to allow the API to check an object 
entirely before doing something important with it. Most of the time, I use 
this feature before inserting or updating a business object into a database. 
I ask, for example, my Customer object about itself : "Do you feel ok ?", if 
the object replies : "no, I have no Name" or something else, I will throw an 
InvalidCustomerException. Because if I carry a Customer object from function 
to function it has to be ok (in memory at least) : it has respect some core 
rules. This kind of defensive programming is really useful when you are 
reading business objects from a database that has not be written by you and 
seldom contains errors you cannot figure easily : you detect theses errors 
immediately. And silently or not, depending what you want.
I will move this classes outside the log package, even if they will stay on 
utility jar.

Marc.