You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Ihor Strutynskyj <ih...@hotmail.com> on 2002/01/08 20:48:04 UTC

CocoonServlet.java patch

Hi all,
This is my first message to this list, so please forgive me if I offend 
someone.
I'm working on integration of Cocoon2 service with Turbine2 and was reading 
CocoonServlet.java. I found several problems in this file. In function 
initLogger() the lines 566 and 567 (CVS version 1.1 2002/01/03) access 
uninitialized variable log (this.log). I think the patch should look like:

--- CocoonServlet.java.orig     Thu Jan  3 04:31:20 2002
+++ CocoonServlet.java  Tue Jan  8 10:50:22 2002
@@ -563,8 +563,8 @@
         if (this.servletContextPath == null) {
             File logSCDir = new File(this.workDir, "log");
             logSCDir.mkdirs();
-            if (log.isWarnEnabled()) {
-                this.log.warn("Setting servlet-context for LogKit to " + 
logSCDir);
+            if (logger.isWarnEnabled()) {
+                logger.warn("Setting servlet-context for LogKit to " + 
logSCDir);
             }
             subcontext.put("context-root", logSCDir.toString());
         } else {

The other thing I would like to mention is:
Why org.apache.log.Logger.log.warn() (or .info(), .error(), .debug()) call 
is always wrapped with if (log.isWarnEnabled()) {}? The 
org.apache.log.Logger has already isXxxxEnabled() wrappers in corresponding 
xxxx() functions. The code would look much simpler without those wrappers.

Ihor.

_________________________________________________________________
Get your FREE download of MSN Explorer at http://explorer.msn.com/intl.asp.


---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org


Re: CocoonServlet.java patch

Posted by Peter Royal <pr...@managingpartners.com>.
On Thursday 10 January 2002 01:46 pm, Santiago Gala wrote:
> debug(String msg) {
>     //current impl
>     }
> debug(String msg1, String msg2) {
>     if( isDebugEnabled() )
>         debug(msg1 + msg2); //check for null?
>     }
> ... and so on.
>
> I feel that 4 String pieces would cover more than 80% of cases (WIth 5
> pieces possibly up to 99%), while keeping user code relatively elegant.
> At least I would prefer it to the alternative of having debug statements
> wrapped in ifs.

While that may seem ideal if you're passing in a simple string, what if one 
of the components in the debug message is the .toString() of an object, which 
internally constructs a long debugging string? With your method above you 
still have to take the hit for constructing that string.
-pete

-- 
peter royal -> proyal@managingpartners.com

---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org


Re: CocoonServlet.java patch

Posted by Santiago Gala <sg...@hisitech.com>.
Peter Royal wrote:

>On Tuesday 08 January 2002 02:48 pm, Ihor Strutynskyj wrote:
>
>>The other thing I would like to mention is:
>>Why org.apache.log.Logger.log.warn() (or .info(), .error(), .debug()) call
>>is always wrapped with if (log.isWarnEnabled()) {}? The
>>org.apache.log.Logger has already isXxxxEnabled() wrappers in corresponding
>>xxxx() functions. The code would look much simpler without those wrappers.
>>
>
>Speed, mainly for debug() statements. If the string you want to log is 
>expensive to create, you can skip the creation by testing the loglevel first.
>-pete
>
I don't know it it looks completely ugly to you, but a trick can be to 
define logger methods as follows:

debug(String msg) {
    //current impl
    }
debug(String msg1, String msg2) {
    if( isDebugEnabled() )
        debug(msg1 + msg2); //check for null?
    }
... and so on.

I feel that 4 String pieces would cover more than 80% of cases (WIth 5 
pieces possibly up to 99%), while keeping user code relatively elegant. 
At least I would prefer it to the alternative of having debug statements 
wrapped in ifs.



---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org


Re: CocoonServlet.java patch

Posted by Peter Royal <pr...@managingpartners.com>.
On Tuesday 08 January 2002 02:48 pm, Ihor Strutynskyj wrote:
> The other thing I would like to mention is:
> Why org.apache.log.Logger.log.warn() (or .info(), .error(), .debug()) call
> is always wrapped with if (log.isWarnEnabled()) {}? The
> org.apache.log.Logger has already isXxxxEnabled() wrappers in corresponding
> xxxx() functions. The code would look much simpler without those wrappers.

Speed, mainly for debug() statements. If the string you want to log is 
expensive to create, you can skip the creation by testing the loglevel first.
-pete

-- 
peter royal -> proyal@managingpartners.com

---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org


RE: CocoonServlet.java patch

Posted by Carsten Ziegeler <cz...@s-und-n.de>.
Hi Ihor,

thanks for your patch - it's applied.

Carsten

> -----Original Message-----
> From: Ihor Strutynskyj [mailto:ihor42@hotmail.com]
> Sent: Tuesday, January 08, 2002 8:48 PM
> To: cocoon-dev@xml.apache.org
> Subject: CocoonServlet.java patch
>
>
> Hi all,
> This is my first message to this list, so please forgive me if I offend
> someone.
> I'm working on integration of Cocoon2 service with Turbine2 and
> was reading
> CocoonServlet.java. I found several problems in this file. In function
> initLogger() the lines 566 and 567 (CVS version 1.1 2002/01/03) access
> uninitialized variable log (this.log). I think the patch should look like:
>
> --- CocoonServlet.java.orig     Thu Jan  3 04:31:20 2002
> +++ CocoonServlet.java  Tue Jan  8 10:50:22 2002
> @@ -563,8 +563,8 @@
>          if (this.servletContextPath == null) {
>              File logSCDir = new File(this.workDir, "log");
>              logSCDir.mkdirs();
> -            if (log.isWarnEnabled()) {
> -                this.log.warn("Setting servlet-context for LogKit to " +
> logSCDir);
> +            if (logger.isWarnEnabled()) {
> +                logger.warn("Setting servlet-context for LogKit to " +
> logSCDir);
>              }
>              subcontext.put("context-root", logSCDir.toString());
>          } else {
>
> The other thing I would like to mention is:
> Why org.apache.log.Logger.log.warn() (or .info(), .error(),
> .debug()) call
> is always wrapped with if (log.isWarnEnabled()) {}? The
> org.apache.log.Logger has already isXxxxEnabled() wrappers in
> corresponding
> xxxx() functions. The code would look much simpler without those wrappers.
>
> Ihor.
>
> _________________________________________________________________
> Get your FREE download of MSN Explorer at
> http://explorer.msn.com/intl.asp.
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
> For additional commands, email: cocoon-dev-help@xml.apache.org
>


---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org