1 Reply Latest reply on Oct 3, 2008 7:18 AM by Ales Justin

    Abstractions, reuse and bad examples

    Adrian Brock Master

      Here's an example of a bad abstraction I found by accident.

      This is actually from the tests so its not too bad, but see below.

      This could obviously be made more useful to others with the following changes;

      public class SearchDeployer extends AbstractDeployer
      {
       private URL[] urls;
      
      + private String prefix;
      
      + private String suffix;
      
       public SearchDeployer()
       {
       setStage(DeploymentStages.REAL);
       }
      
       public String getPrefix()
       {
       return prefix;
       }
      
       public void setPrefix(String prefix)
       {
       this.prefix = prefix;
       }
      
       public String getSuffix()
       {
       return suffix;
       }
      
       public void setSuffix(String suffix)
       {
       this.suffix = suffix;
       }
      
       public void deploy(DeploymentUnit unit) throws DeploymentException
       {
       try
       {
      - urls = Classpath.search(unit.getClassLoader(), "META-INF/", ".taglib.xml");
      + urls = Classpath.search(unit.getClassLoader(), prefix, suffix);
       }
       catch (IOException e)
       {
      - DeploymentException.rethrowAsDeploymentException("Error doing facelets search", e);
      + DeploymentException.rethrowAsDeploymentException("Error doing search prefix=" + prefix + " suffix=" + suffix, e);
       }
       }
      
       public void undeploy(DeploymentUnit unit)
       {
       urls = null;
       }
      
       public URL[] getUrls()
       {
       return urls;
       }
      }
      


      That way the facelets deployer (which is obviously where this comes from?)
      can extend this SearchDeployer and reuse generic tested code.

      This argument is further enhanced because if you look at what
      ClassPath.search() it is wrong,
      it isn't making reference to the ClassLoadingMetaData/Module
      so it will ignore any filters.

      Has this code been copied somewhere else?

      It's generally a bad idea to show the wrong way to do things in tests
      because people copy what they find thinking that's the way it is supposed to work. :-)

        • 1. Re: Abstractions, reuse and bad examples
          Ales Justin Master

           

          "adrian@jboss.org" wrote:

          This is actually from the tests so its not too bad, but see below.

          This could obviously be made more useful to others with the following changes;

          This deployer is only there to trigger Classpath::search w/o the real usage of facelets.

          It shouldn't even be considered as a true deployer test/example.
          Hence the only good/proper change would be to the javadocs,
          stating its purpose, 'warning' against reuse. ;-)

          "adrian@jboss.org" wrote:

          This argument is further enhanced because if you look at what
          ClassPath.search() it is wrong,
          it isn't making reference to the ClassLoadingMetaData/Module
          so it will ignore any filters.

          Has this code been copied somewhere else?

          It's the Classpath class we're interested in.
          And the fix to understand vfs and nested jars / resources.

          This patch was ported to main facelets code.
          So, there is no way to know anything about CLMD.

          I agree with you that it's wrong,
          but tell this to n external frameworks that make bad url assumptions. :-)

          "adrian@jboss.org" wrote:

          It's generally a bad idea to show the wrong way to do things in tests
          because people copy what they find thinking that's the way it is supposed to work. :-)

          I'll fix the javadocs on this. ;-)