You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Jacques Le Roux <ja...@les7arts.com> on 2014/09/02 11:43:43 UTC

Re: svn commit: r1621804 - in /ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp: control/RequestHandler.java event/EventFactory.java event/RomeEventHandler.java view/ViewFactory.java

Inline

Le 01/09/2014 16:26, jacopoc@apache.org a écrit :
> Author: jacopoc
> Date: Mon Sep  1 14:26:54 2014
> New Revision: 1621804
>
> URL: http://svn.apache.org/r1621804
> Log:
> A series of clean-ups and modifications to RequestHandler, EventFactory and View Factory to make then (mostly) immutable and eliminate some potential concurrency issues that could happen at initialization. This is just a first pass.
>
> Modified:
>      ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java
>      ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/EventFactory.java
>      ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/RomeEventHandler.java
>      ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/view/ViewFactory.java
>
> Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java?rev=1621804&r1=1621803&r2=1621804&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java (original)
> +++ ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java Mon Sep  1 14:26:54 2014
> @@ -69,29 +69,29 @@ import org.owasp.esapi.errors.EncodingEx
>   public class RequestHandler {
>   
>       public static final String module = RequestHandler.class.getName();
> -    private boolean throwRequestHandlerExceptionOnMissingLocalRequest = UtilProperties.propertyValueEqualsIgnoreCase(
> +    private static final boolean throwRequestHandlerExceptionOnMissingLocalRequest = UtilProperties.propertyValueEqualsIgnoreCase(
>               "requestHandler.properties", "throwRequestHandlerExceptionOnMissingLocalRequest", "Y");
> -    private String statusCodeString = UtilProperties.getPropertyValue("requestHandler.properties", "status-code", "302");

I don't see a need to make those finals. They are cached and cost at maximum hundreds of bytes.
Having them cached allows to dynamically change them, which can be handy sometimes (general remark for other possible cases)

Jacques

Re: svn commit: r1621804 - in /ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp: control/RequestHandler.java event/EventFactory.java event/RomeEventHandler.java view/ViewFactory.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
Le 02/09/2014 11:53, Jacopo Cappellato a écrit :
> On Sep 2, 2014, at 11:43 AM, Jacques Le Roux <ja...@les7arts.com> wrote:
>> I don't see a need to make those finals. They are cached and cost at maximum hundreds of bytes.
>> Having them cached allows to dynamically change them, which can be handy sometimes (general remark for other possible cases)
>>
>> Jacques
> These fields were not modifiable even before my changes; the final keyword I have added didn't change the external behavior of them. However in general, if some field can change and the class can be used by different threads then you have to make sure the class is thread safe (and this is expensive): this is the main advantage of having (when possible) immutable classes, they are thread safe at no cost. This is not a matter of memory usage.

Indeed, forgot the _REQUEST_HANDLER_ servletContext  attribute

Jacques

>
> Jacopo

Re: svn commit: r1621804 - in /ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp: control/RequestHandler.java event/EventFactory.java event/RomeEventHandler.java view/ViewFactory.java

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
On Sep 2, 2014, at 11:43 AM, Jacques Le Roux <ja...@les7arts.com> wrote:
> 
> I don't see a need to make those finals. They are cached and cost at maximum hundreds of bytes.
> Having them cached allows to dynamically change them, which can be handy sometimes (general remark for other possible cases)
> 
> Jacques

These fields were not modifiable even before my changes; the final keyword I have added didn't change the external behavior of them. However in general, if some field can change and the class can be used by different threads then you have to make sure the class is thread safe (and this is expensive): this is the main advantage of having (when possible) immutable classes, they are thread safe at no cost. This is not a matter of memory usage.

Jacopo