[jira] [Commented] (LANG-1144) Multiple calls of org.apache.commons.lang3.concurrent.LazyInitializer.initialize() are possible
[
https://issues.apache.org/jira/browse/LANG-1144?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15599914#comment-15599914
]
Oliver Heger commented on LANG-1144:
The patch looks good to me, but I would propose to make the special noInit
object even *static*.
Regarding exception handling: I think, the currently implemented solution is
in-line with the approach taken by the JDK. See for instance {{Future.get()}},
which throws an {{ExecutionException}}. There is also some support for the
exception type in the {{ConcurrentUtils}} class.
> 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
>Assignee: Gary Gregory
> Attachments: 0001-LANG-1144-allow-nulls-as-return-value.patch,
> commons-lang.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 lazyIinit = new LazyInitializer() {
> @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)
[jira] [Commented] (LANG-1144) Multiple calls of org.apache.commons.lang3.concurrent.LazyInitializer.initialize() are possible
[
https://issues.apache.org/jira/browse/LANG-1144?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15598136#comment-15598136
]
Gary Gregory commented on LANG-1144:
I wish I had paid more attention when this class was added because
{{org.apache.commons.lang3.concurrent.LazyInitializer.get()}} throwing a custom
{{ConcurrentException}} does not make sense to me either. That's because if I
catch an error while I am building my object, it might have nothing to do with
concurrency. The fact that the LI class helps me deal with concurrency matters,
yes. This should be an {{IllegalStateException}}. We could change this for 4.0
I suppose.
> 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
>Assignee: Gary Gregory
> 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 lazyIinit = new LazyInitializer() {
> @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)
[jira] [Commented] (LANG-1144) Multiple calls of org.apache.commons.lang3.concurrent.LazyInitializer.initialize() are possible
[
https://issues.apache.org/jira/browse/LANG-1144?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15598131#comment-15598131
]
Gary Gregory commented on LANG-1144:
This seems like a valid bug report to me.
I do not think it reasonable to ask user to create a "special" null value for a
JDBC Connection or Statement, for example:
{code:java}
LazyInitializer
LazyInitializer
{code}
Now, I have to go and implement a "null" implementation of all the interfaces I
want to use? That's not acceptable.
Furthermore, what if I have a {{ComplexSomething}} class with many private
final fields that get initialized on construction to create a valid object? I
now have to change the class to allow of all null values? Not acceptable either.
> 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
> 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 lazyIinit = new LazyInitializer() {
> @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)
[jira] [Commented] (LANG-1144) Multiple calls of org.apache.commons.lang3.concurrent.LazyInitializer.initialize() are possible
[
https://issues.apache.org/jira/browse/LANG-1144?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=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 LazyInitializerObject lazyIinit = new LazyInitializerObject() {
@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)
