Re: RFR: 8264326: Modernize javax.script.ScriptEngineManager and related classes' implementation [v2]
On Tue, 30 Mar 2021 12:43:18 GMT, Athijegannathan Sundararajan wrote: >> Attila Szegedi has refreshed the contents of this pull request, and previous >> commits have been removed. The incremental views will show differences >> compared to the previous content of the PR. The pull request contains 13 new >> commits since the last revision: >> >> - Tidy >> - require non null in SimpleBindings >> - Simplify SimpleScriptContext.scopes >> - Mark fields final where possible >> - Deduplicate registerEngineXxx methods >> - Misc tidying >> - Deduplicate exception reporting >> - Lambdify >> - Mark fields as final; eliminate now unnecessary init() method. >> - Deduplicate engine creation and setup code >> - ... and 3 more: >> https://git.openjdk.java.net/jdk/compare/f378f350...56d89eb2 > > Marked as reviewed by sundar (Reviewer). Thank you for the review @sundararajana! Much appreciated. - PR: https://git.openjdk.java.net/jdk/pull/3229
Re: RFR: 8264326: Modernize javax.script.ScriptEngineManager and related classes' implementation [v2]
On Sun, 28 Mar 2021 13:38:51 GMT, Attila Szegedi wrote: >> I noticed that `javax.script.ScriptEngineManager` `getEngineByXxx` methods >> had a lot of code duplication among themselves, and even within each method. >> I refactored them into a modern unified implementation. While there I also >> took the opportunity to introduce `Objects.requireNonNull` in place of null >> checks followed by NPE throws, mark private fields final where possible, use >> lambdas for `doPrivileged` block, use `List.of` and `List.copyOf` where >> possible, and generally sanitize/deduplicate. > > Attila Szegedi has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. The pull request contains 13 new > commits since the last revision: > > - Tidy > - require non null in SimpleBindings > - Simplify SimpleScriptContext.scopes > - Mark fields final where possible > - Deduplicate registerEngineXxx methods > - Misc tidying > - Deduplicate exception reporting > - Lambdify > - Mark fields as final; eliminate now unnecessary init() method. > - Deduplicate engine creation and setup code > - ... and 3 more: > https://git.openjdk.java.net/jdk/compare/f378f350...56d89eb2 Marked as reviewed by sundar (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3229
Re: RFR: 8264326: Modernize javax.script.ScriptEngineManager and related classes' implementation [v2]
> I noticed that `javax.script.ScriptEngineManager` `getEngineByXxx` methods > had a lot of code duplication among themselves, and even within each method. > I refactored them into a modern unified implementation. While there I also > took the opportunity to introduce `Objects.requireNonNull` in place of null > checks followed by NPE throws, mark private fields final where possible, use > lambdas for `doPrivileged` block, use `List.of` and `List.copyOf` where > possible, and generally sanitize/deduplicate. Attila Szegedi has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains 13 new commits since the last revision: - Tidy - require non null in SimpleBindings - Simplify SimpleScriptContext.scopes - Mark fields final where possible - Deduplicate registerEngineXxx methods - Misc tidying - Deduplicate exception reporting - Lambdify - Mark fields as final; eliminate now unnecessary init() method. - Deduplicate engine creation and setup code - ... and 3 more: https://git.openjdk.java.net/jdk/compare/f378f350...56d89eb2 - Changes: - all: https://git.openjdk.java.net/jdk/pull/3229/files - new: https://git.openjdk.java.net/jdk/pull/3229/files/f378f350..56d89eb2 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3229=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3229=00-01 Stats: 6 lines in 1 file changed: 4 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/3229.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3229/head:pull/3229 PR: https://git.openjdk.java.net/jdk/pull/3229
Re: RFR: 8264326: Modernize javax.script.ScriptEngineManager and related classes' implementation [v2]
On Sat, 27 Mar 2021 15:46:36 GMT, Alan Bateman wrote:
>> Attila Szegedi has refreshed the contents of this pull request, and previous
>> commits have been removed. The incremental views will show differences
>> compared to the previous content of the PR. The pull request contains 13 new
>> commits since the last revision:
>>
>> - Tidy
>> - require non null in SimpleBindings
>> - Simplify SimpleScriptContext.scopes
>> - Mark fields final where possible
>> - Deduplicate registerEngineXxx methods
>> - Misc tidying
>> - Deduplicate exception reporting
>> - Lambdify
>> - Mark fields as final; eliminate now unnecessary init() method.
>> - Deduplicate engine creation and setup code
>> - ... and 3 more:
>> https://git.openjdk.java.net/jdk/compare/f378f350...56d89eb2
>
> src/java.scripting/share/classes/javax/script/ScriptEngineManager.java line
> 215:
>
>> 213: }
>> 214:
>> 215: private ScriptEngine getEngineBy(String selector, Map> ScriptEngineFactory> associations, Function> List> valuesFn) {
>
> No objection to do some modernization of this code but probably best to avoid
> introducing overly long lines as it is impossible to see the changes with
> side-by-side diffs.
Fair enough, I reformatted lines longer than 120 characters.
-
PR: https://git.openjdk.java.net/jdk/pull/3229
