commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jacopo Cappellato (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (POOL-280) Code cleanups for GenericObjectPool.borrowObject(...)
Date Mon, 05 Jan 2015 07:20:35 GMT

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

Jacopo Cappellato commented on POOL-280:
----------------------------------------

I couldn't find in my local modifications the changes to DefaultPooledObject that I was referring
to in #1, please ignore them.
As regards the rationale about the chafe in GOP, the only real reason is that the code block:
{quote}
                p = idleObjects.pollFirst();
                if (p == null) {
                    p = create();
                    if (p != null) {
                        create = true;
                    }
                }
{quote}
is the same in the two if/else blocks of the if statement that tests the blockWhenExhausted
variable: with my proposed change I have moved them outside of the if/else statement, reducing
some duplication of code.
No functional changes, the only real rational is to slightly simplify the readability of this
code blocks.


> Code cleanups for GenericObjectPool.borrowObject(...)
> -----------------------------------------------------
>
>                 Key: POOL-280
>                 URL: https://issues.apache.org/jira/browse/POOL-280
>             Project: Commons Pool
>          Issue Type: Improvement
>    Affects Versions: 2.2
>            Reporter: Jacopo Cappellato
>            Priority: Minor
>             Fix For: 2.5
>
>         Attachments: POOL-280.patch
>
>
> In the attached patch you will find 2 minor code cleanups (no functional changes) in
order to slightly simplify the code:
> # removed an if block that was unnecessary from DefaultPooledObject.deallocate()
> # removed some duplicate code from the two blocks of an if-else statement in GenericObjectPool.borrowObject(...);
the original code was:
> {code}
>         while (p == null) {
>             create = false;
>             if (blockWhenExhausted) {
>                 p = idleObjects.pollFirst();
>                 if (p == null) {
>                     p = create();
>                     if (p != null) {
>                         create = true;
>                     }
>                 }
>                 if (p == null) {
>                     if (borrowMaxWaitMillis < 0) {
>                         p = idleObjects.takeFirst();
>                     } else {
>                         p = idleObjects.pollFirst(borrowMaxWaitMillis,
>                                 TimeUnit.MILLISECONDS);
>                     }
>                 }
>                 if (p == null) {
>                     throw new NoSuchElementException(
>                             "Timeout waiting for idle object");
>                 }
>                 if (!p.allocate()) {
>                     p = null;
>                 }
>             } else {
>                 p = idleObjects.pollFirst();
>                 if (p == null) {
>                     p = create();
>                     if (p != null) {
>                         create = true;
>                     }
>                 }
>                 if (p == null) {
>                     throw new NoSuchElementException("Pool exhausted");
>                 }
>                 if (!p.allocate()) {
>                     p = null;
>                 }
>             }
>             ...
>         }
> {code}
> and the new code is:
> {code}
>         while (p == null) {
>             create = false;
>             p = idleObjects.pollFirst();
>             if (p == null) {
>                 p = create();
>                 if (p != null) {
>                     create = true;
>                 }
>             }
>             if (p == null) {
>                 if (blockWhenExhausted) {
>                     if (borrowMaxWaitMillis < 0) {
>                         p = idleObjects.takeFirst();
>                     } else {
>                         p = idleObjects.pollFirst(borrowMaxWaitMillis,
>                                 TimeUnit.MILLISECONDS);
>                     }
>                     if (p == null) {
>                         throw new NoSuchElementException(
>                                 "Timeout waiting for idle object");
>                     }
>                 } else {
>                     throw new NoSuchElementException("Pool exhausted");
>                 }
>             }
>             if (!p.allocate()) {
>                 p = null;
>             }
>             ...
>         }
> {code}



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

Mime
View raw message