> On Aug. 17, 2017, 5:35 p.m., Jared Stewart wrote:
> > geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java
> > Lines 2017 (patched)
> > <https://reviews.apache.org/r/61701/diff/1/?file=1798850#file1798850line2020>
> >
> > I think this double-ternary might be easier to read if we split it up
> > into a few separate statements.
>
> Ken Howe wrote:
> Agree with you that the double ternary sin't the most readable construct.
> However, I ended up using it because the `this()` call has to be the first
> statement in the constructor. The conditionals guard against NPEs from the
> possible locations where this error constructor is called.
>
> I'm open to other refactoring suggestions to make this easier to
> understand.
I would suggest this sort of pattern:
```
public LocatorState(final LocatorLauncher launcher, final Status status,
final String errorMessage) {
this(status, // status
errorMessage, // statusMessage
System.currentTimeMillis(), // timestamp
getLocatorLocation(launcher),
null, // pid
0L, // uptime
launcher.getWorkingDirectory(), // workingDirectory
ManagementFactory.getRuntimeMXBean().getInputArguments(), //
jvmArguments
null, // classpath
GemFireVersion.getGemFireVersion(), // gemfireVersion
System.getProperty("java.version"), // javaVersion
null, // logFile
launcher.getBindAddressAsString(), // host
launcher.getPortAsString(), // port
null);// memberName
}
private static String getLocatorLocation(LocatorLauncher launcher) {
return launcher.getPort() == null ? launcher.getId()
: HostUtils.getLocatorId((launcher.getBindAddress() == null)
? HostUtils.getLocalHost() :
launcher.getBindAddress().getCanonicalHostName(),
launcher.getPort());
}
```
where then you can split up them implementation of `getLocatorLocation` into
several separate lines.
- Jared
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61701/#review183145
-----------------------------------------------------------
On Aug. 16, 2017, 9:21 p.m., Ken Howe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61701/
> -----------------------------------------------------------
>
> (Updated Aug. 16, 2017, 9:21 p.m.)
>
>
> Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Kirk Lund,
> and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Updated tests for changes in the error constructors for ServerState and
> LocatorState.
>
> Minor spelling corrections.
>
> I've updated the fix for GEODE-3277 and rebased on top of the @klund's recent
> process controller updates
>
>
> Diffs
> -----
>
>
> geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StatusLocatorRealGfshTest.java
> 3a98373938e3de21da6badcf460dae3648218ac6
> geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java
> 83c1ab533e3dea323a8a99f7002b9464a54dfc25
> geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java
> ae64691605130c9b212a3a33bb65ae37b28af02b
>
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/lifecycle/GfshStatusCommandsIntegrationTest.java
> dd5841f4cffca38da07a11f381cf4174d7264349
>
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java
> e7f17ef208a1708f385c7c4041affb70fd309a4c
>
>
> Diff: https://reviews.apache.org/r/61701/diff/1/
>
>
> Testing
> -------
>
> Precheckin is in progress.
>
>
> Thanks,
>
> Ken Howe
>
>
