RFR: 8263894: Convert defaultPrinter and printers fields to local variables

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8263894: Convert defaultPrinter and printers fields to local variables

Alexey Ivanov-2
`PrintServiceLookupProvider` has `defaultPrinter` and `printers` fields but they are used only in `getDefaultPrintService()` and `refreshServices()` correspondingly. Thus these two fields can be converted to local variables in the corresponding methods.

-------------

Commit messages:
 - 8263894: Convert defaultPrinter and printers fields to local variables

Changes: https://git.openjdk.java.net/jdk/pull/3095/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3095&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8263894
  Stats: 5 lines in 1 file changed: 0 ins; 2 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3095.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3095/head:pull/3095

PR: https://git.openjdk.java.net/jdk/pull/3095
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8263894: Convert defaultPrinter and printers fields to local variables

Phil Race
On Fri, 19 Mar 2021 20:35:03 GMT, Alexey Ivanov <[hidden email]> wrote:

> `PrintServiceLookupProvider` has `defaultPrinter` and `printers` fields but they are used only in `getDefaultPrintService()` and `refreshServices()` correspondingly. Thus these two fields can be converted to local variables in the corresponding methods.

Why did you delete the comments ?

-------------

PR: https://git.openjdk.java.net/jdk/pull/3095
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8263894: Convert defaultPrinter and printers fields to local variables [v2]

Alexey Ivanov-2
In reply to this post by Alexey Ivanov-2
> `PrintServiceLookupProvider` has `defaultPrinter` and `printers` fields but they are used only in `getDefaultPrintService()` and `refreshServices()` correspondingly. Thus these two fields can be converted to local variables in the corresponding methods.

Alexey Ivanov has updated the pull request incrementally with one additional commit since the last revision:

  Bring back the comment for printServices

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3095/files
  - new: https://git.openjdk.java.net/jdk/pull/3095/files/cfe460c2..2b98fec2

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3095&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3095&range=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3095.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3095/head:pull/3095

PR: https://git.openjdk.java.net/jdk/pull/3095
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8263894: Convert defaultPrinter and printers fields to local variables [v2]

Alexey Ivanov-2
In reply to this post by Phil Race
On Fri, 19 Mar 2021 21:05:57 GMT, Phil Race <[hidden email]> wrote:

> Why did you delete the comments ?

The comments highlighted the difference between what is stored in `printers` and in `printServices`: _excludes_ vs. _includes_ the default printer. However, according to the code, `printers` _included_ the default printer too.

Now there's only `printServices` which contains all print services. I decided the comment didn't have much value now. Yet, upon thinking about it, the comment for `printServices` still provides additional insight: the array also contains the `defaultPrintService`.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3095
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8263894: Convert defaultPrinter and printers fields to local variables [v2]

Phil Race
In reply to this post by Alexey Ivanov-2
On Fri, 19 Mar 2021 21:28:54 GMT, Alexey Ivanov <[hidden email]> wrote:

>> `PrintServiceLookupProvider` has `defaultPrinter` and `printers` fields but they are used only in `getDefaultPrintService()` and `refreshServices()` correspondingly. Thus these two fields can be converted to local variables in the corresponding methods.
>
> Alexey Ivanov has updated the pull request incrementally with one additional commit since the last revision:
>
>   Bring back the comment for printServices

Marked as reviewed by prr (Reviewer).

-------------

PR: https://git.openjdk.java.net/jdk/pull/3095
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8263894: Convert defaultPrinter and printers fields to local variables [v2]

Alexander Zvegintsev-2
In reply to this post by Alexey Ivanov-2
On Fri, 19 Mar 2021 21:28:54 GMT, Alexey Ivanov <[hidden email]> wrote:

>> `PrintServiceLookupProvider` has `defaultPrinter` and `printers` fields but they are used only in `getDefaultPrintService()` and `refreshServices()` correspondingly. Thus these two fields can be converted to local variables in the corresponding methods.
>
> Alexey Ivanov has updated the pull request incrementally with one additional commit since the last revision:
>
>   Bring back the comment for printServices

Marked as reviewed by azvegint (Reviewer).

-------------

PR: https://git.openjdk.java.net/jdk/pull/3095
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8263894: Convert defaultPrinter and printers fields to local variables [v2]

Alexander Zuev-3
In reply to this post by Alexey Ivanov-2
On Fri, 19 Mar 2021 21:28:54 GMT, Alexey Ivanov <[hidden email]> wrote:

>> `PrintServiceLookupProvider` has `defaultPrinter` and `printers` fields but they are used only in `getDefaultPrintService()` and `refreshServices()` correspondingly. Thus these two fields can be converted to local variables in the corresponding methods.
>
> Alexey Ivanov has updated the pull request incrementally with one additional commit since the last revision:
>
>   Bring back the comment for printServices

Marked as reviewed by kizune (Reviewer).

-------------

PR: https://git.openjdk.java.net/jdk/pull/3095
Reply | Threaded
Open this post in threaded view
|

Integrated: 8263894: Convert defaultPrinter and printers fields to local variables

Alexey Ivanov-2
In reply to this post by Alexey Ivanov-2
On Fri, 19 Mar 2021 20:35:03 GMT, Alexey Ivanov <[hidden email]> wrote:

> `PrintServiceLookupProvider` has `defaultPrinter` and `printers` fields but they are used only in `getDefaultPrintService()` and `refreshServices()` correspondingly. Thus these two fields can be converted to local variables in the corresponding methods.

This pull request has now been integrated.

Changeset: 840ab7bf
Author:    Alexey Ivanov <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/840ab7bf
Stats:     4 lines in 1 file changed: 0 ins; 2 del; 2 mod

8263894: Convert defaultPrinter and printers fields to local variables

Reviewed-by: prr, azvegint, kizune

-------------

PR: https://git.openjdk.java.net/jdk/pull/3095