Re: cvs commit: incubator-geronimo/sandbox/webdav/src/test/org/apache/geronimo/datastore/impl/remote RemoteUseCaseTest.java

[Jacek Laskowski <jl...@apache.org>]

gdamour@apache.org wrote:
> gdamour     2004/02/29 05:14:11
...
>   1.1                  incubator-geronimo/sandbox/webdav/src/test/org/apache/geronimo/datastore/Util.java
...
>   package org.apache.geronimo.datastore;
>   
>   import java.io.File;
>   import java.io.IOException;
>   
>   /**
>    *
>    * @version $Revision: 1.1 $ $Date: 2004/02/29 13:14:11 $
>    */
>   public class Util {
>       
>       public static void recursiveDelete(File aRoot) throws IOException {

The method already exists in 
./modules/deployment/src/java/org/apache/geronimo/deployment/util/FileUtil.java. 


That rises a very interesting question - where do we put methods/classes 
that are common to several modules?

>           Exception ex = null;
>           try {
>               dirtyMarker.setIsDelete(true);
>           } catch (IllegalArgumentException e) {
>               ex = e;
>           }
>           assertNotNull("Transition should ne impossible", ex);

Why do you check whether or not ex is not null that way? Unless I'm 
mistaken, it boils down to the following junit pattern:

  try
  {
    dirtyMarker.setIsDelete(true);
    fail("Transition should not be impossible; IllegalArgumentException 
expected");
  } catch (IllegalArgumentException expected) {
  }

There's also a typo in the message - 'should ne impossible' == 'should 
not be possible'.

Cheers,
Jacek

Re: cvs commit: incubator-geronimo/sandbox/webdav/src/test/org/apache/geronimo/datastore/impl/remote RemoteUseCaseTest.java

[Jacek Laskowski <jl...@apache.org>]

Dain Sundstrom wrote:

>> That rises a very interesting question - where do we put  
>> methods/classes that are common to several modules?
> 
> 
> I'd vote no.  It just leads to a highly coupled system, and our commons  
> module depends on to much other stuff.  If we were to get our commons  
> module stand-alone, then I might change my mind.

I thought similarly, but what do we have instead - copying files between 
modules, just to not tie them up together is not the way to follow, 
isn't it?

Hopefully in that particular case moving the method to common module is 
the solution as it doesn't incur any further dependencies. Other copies 
would be sorted out as they pop up.

> -dain

Jacek

Re: cvs commit: incubator-geronimo/sandbox/webdav/src/test/org/apache/geronimo/datastore/impl/remote RemoteUseCaseTest.java

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

On Feb 29, 2004, at 11:14 AM, Jacek Laskowski wrote:

> gdamour@apache.org wrote:
>> gdamour     2004/02/29 05:14:11
> ...
>>   1.1                   
>> incubator-geronimo/sandbox/webdav/src/test/org/apache/geronimo/ 
>> datastore/Util.java
> ...
>>   package org.apache.geronimo.datastore;
>>     import java.io.File;
>>   import java.io.IOException;
>>     /**
>>    *
>>    * @version $Revision: 1.1 $ $Date: 2004/02/29 13:14:11 $
>>    */
>>   public class Util {
>>             public static void recursiveDelete(File aRoot) throws  
>> IOException {
>
> The method already exists in  
> ./modules/deployment/src/java/org/apache/geronimo/deployment/util/ 
> FileUtil.java.
>
> That rises a very interesting question - where do we put  
> methods/classes that are common to several modules?

I'd vote no.  It just leads to a highly coupled system, and our commons  
module depends on to much other stuff.  If we were to get our commons  
module stand-alone, then I might change my mind.

-dain