[jira] [Commented] (LANG-1144) Multiple calls of org.apache.commons.lang3.concurrent.LazyInitializer.initialize() are possible

["Oliver Heger (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/LANG-1144?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14582444#comment-14582444 ] 

Oliver Heger commented on LANG-1144:
------------------------------------

I do not think that this is really a critical problem. The use case that a longer and complex calculation yields a null result at the end seems a bit odd to me.

And - as you have a demonstrated in your patch - there is an easy work-around for the problem: Just use a special null object or a wrapper type like MutableObject or the new Java 8 type Optional. I would be reluctant to add such means to the current implementation because this increases complexity and is (slightly) less efficient. This is IMHO a too high price for such a corner use case.

What should be done in any case is to add a note to the Javadocs describing this problem.

> Multiple calls of org.apache.commons.lang3.concurrent.LazyInitializer.initialize() are possible
> -----------------------------------------------------------------------------------------------
>
>                 Key: LANG-1144
>                 URL: https://issues.apache.org/jira/browse/LANG-1144
>             Project: Commons Lang
>          Issue Type: Bug
>          Components: lang.concurrent.*
>    Affects Versions: 3.4
>         Environment: Java 1.8 on Windows 7 x64
>            Reporter: Waldemar Maier
>            Priority: Critical
>         Attachments: 0001-LANG-1144-allow-nulls-as-return-value.patch
>
>
> It is possible to create a construct, that allows multiple calls of LazyInitializer.initialize, when calculations (which can be very expensive) return null as result. 
> In the Javadoc is described that the initialize method will be called only on the first access
> {code:java}
>     /**
>      * Creates and initializes the object managed by this {@code
>      * LazyInitializer}. This method is called by {@link #get()} when the object
>      * is accessed for the first time. An implementation can focus on the
>      * creation of the object. No synchronization is needed, as this is already
>      * handled by {@code get()}.
>      *
>      * @return the managed data object
>      * @throws ConcurrentException if an error occurs during object creation
>      */
>     protected abstract T initialize() throws ConcurrentException;
> {code}
> The Junit Test can be something like this:
> *(fix can be appplied from attached patch-file)*
> {code:java}
> package edu.test;
> import static org.junit.Assert.assertEquals;
> import org.apache.commons.lang3.concurrent.ConcurrentException;
> import org.apache.commons.lang3.concurrent.LazyInitializer;
> import org.junit.Test;
> public class LazyInitializerTest {
>   private int lazyinitCounter = 0;
>   private LazyInitializer<Object> lazyIinit = new LazyInitializer<Object>() {
>     @Override
>     protected Object initialize() throws ConcurrentException {
>       lazyinitCounter++;
>       return doSomeVeryExpensiveOperations();
>     }
>   };
>   
>   
>   private Object doSomeVeryExpensiveOperations() {
>     // do db calls
>     // do some complex math calculations
>     // the result of them all is null
>     return null;
>   }
>   
>   
>   @Test
>   public void testInitialization() throws Exception {
>     lazyIinit.get();
>     lazyIinit.get();
>     assertEquals("Multiple call of LazyInitializer#initialize", 1, lazyinitCounter);
>   }
> }
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)