Re: RFR: 8287834: Add SymbolLookup::or method [v3]

2023-05-22 Thread Maurizio Cimadamore



On 17/05/2023 02:33, John Hendrikx wrote:

SymbolLookup lookUp = name -> library1.find(name)
   .or(() -> library2.find(name))
   .or(() -> loader.find(name));


What you say is true - e.g. the fact that a lookup returns an optional 
can be used to chain multiple lookups as you describe.


There are, however some difference - note that your example only uses 
"captures" symbol lookup variable names - so the full code would be more 
something like:


```java
SymbolLookup library1 = SymbolLookup.libraryLookup("lib1", arena);
SymbolLookup library2 = SymbolLookup.libraryLookup("lib2", arena);
SymbolLookup loader = SymbolLookup.loaderLookup();
SymbolLookup lookUp = name -> library1.find(name)
  .or(() -> library2.find(name))
  .or(() -> loader.find(name));
```

Without the initial setup, we could try to write something like this:

```java
SymbolLookup lookUp = name -> SymbolLookup.libraryLookup("lib1", 
arena).find(name)

  .or(() -> SymbolLookup.libraryLookup("lib2", arena).find(name))
  .or(() -> SymbolLookup.loaderLookup().find(name));
```

But this more compact version has several issues. First, it creates a 
new library lookup on each call to "find" (possibly more). Second, the 
loader in which the lookup is retrieved depends on who calls the "find" 
method.


So, the more compact version is not "like for like" for the more verbose 
option above. And this is due to the fact that, in most cases, 
retrieving a symbol lookup has some side-effects, such as loading a 
library - it is *not* a purely functional process.


With the new method, it becomes like this:

```java
SymbolLookup lookUp =SymbolLookup.libraryLookup("lib1", arena)
  .or(SymbolLookup.libraryLookup("lib2", arena))
  .or(SymbolLookup.loaderLookup());
```

So, not only the new method results in more succinct code (compared to 
the alternatives), but it also combines symbol lookups in the "right 
way", so that the chain of lookups is defined, once and for all, when 
the composite lookup is first created.


Cheers
Maurizio



Re: RFR: 8287834: Add SymbolLookup::or method [v3]

2023-05-16 Thread John Hendrikx
On Tue, 16 May 2023 22:36:51 GMT, Maurizio Cimadamore  
wrote:

>> This patch adds a simpler method for composing symbol lookups. It is common 
>> for clients to chain multiple symbol lookups together, e.g. to find a symbol 
>> in multiple libraries.
>> 
>> A new instance method, namely `SymbolLookup::or` is added, which first 
>> searches a symbol in the first lookup, and, if that fails, proceeds to 
>> search the symbol in the provided lookup.
>> 
>> We have considered alternatives to express this, such as a static factory 
>> `SymbolLookup::ofComposite` but settled on this because of the similarity 
>> with `Optional::or`.
>
> Maurizio Cimadamore has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Tweak javadoc

Isn't it already possible to easily create a composed `SymbolLookup`?

 SymbolLookup lookUp = name -> library1.find(name)
  .or(() -> library2.find(name))
  .or(() -> loader.find(name));

The proposed method may be a bit nicer, but it is sort of duplicating what 
`Optional::or` was intended for.

-

PR Comment: https://git.openjdk.org/jdk/pull/13954#issuecomment-1550560151


Re: RFR: 8287834: Add SymbolLookup::or method [v3]

2023-05-16 Thread Maurizio Cimadamore
> This patch adds a simpler method for composing symbol lookups. It is common 
> for clients to chain multiple symbol lookups together, e.g. to find a symbol 
> in multiple libraries.
> 
> A new instance method, namely `SymbolLookup::or` is added, which first 
> searches a symbol in the first lookup, and, if that fails, proceeds to search 
> the symbol in the provided lookup.
> 
> We have considered alternatives to express this, such as a static factory 
> `SymbolLookup::ofComposite` but settled on this because of the similarity 
> with `Optional::or`.

Maurizio Cimadamore has updated the pull request incrementally with one 
additional commit since the last revision:

  Tweak javadoc

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/13954/files
  - new: https://git.openjdk.org/jdk/pull/13954/files/27e82bb6..76578973

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=13954=02
 - incr: https://webrevs.openjdk.org/?repo=jdk=13954=01-02

  Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/13954.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/13954/head:pull/13954

PR: https://git.openjdk.org/jdk/pull/13954