> On Feb. 12, 2016, 6:27 p.m., Jason Huynh wrote: > > gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/JMXDataUpdater.java, > > line 565 > > <https://reviews.apache.org/r/43507/diff/2/?file=1240866#file1240866line565> > > > > Not sure if it matters, but would it be better to pull out > > memMBean.getKeyProperty(PulseConstants.MBEAN_KEY_PROPERTY_SERVICE) into a > > local variable and using that in all the else if statements or switch > > statement?
Yeah, it would be much better. I guess the original code was written pre-java1.7 when switch doesn't work with String? There are lots of code in this file that looks like this. We should do that as part of a larger refactoring effort. > On Feb. 12, 2016, 6:27 p.m., Jason Huynh wrote: > > gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/JMXDataUpdater.java, > > line 1850 > > <https://reviews.apache.org/r/43507/diff/2/?file=1240866#file1240866line1850> > > > > same suggestion here incase attribute.getName() does any type of > > calculation instead of a simple field get. Agree. - Jinmei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43507/#review119054 ----------------------------------------------------------- On Feb. 12, 2016, 6:15 p.m., Jinmei Liao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43507/ > ----------------------------------------------------------- > > (Updated Feb. 12, 2016, 6:15 p.m.) > > > Review request for geode. > > > Repository: geode > > > Description > ------- > > GEODE-954: retrieve the hostnameForClients attribute from the mbean and add a > column in the member list view. > > > Diffs > ----- > > > gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/Cluster.java > a65a07ef922df83e33e8f785db9dcf1256283747 > > gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/JMXDataUpdater.java > 1cd5fd7b7d976683c09f410e558e0d23c6548cd5 > > gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/data/PulseConstants.java > 9c5732e81e010e73c5a445fc8880b5b8d139e5a9 > > gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/service/ClusterMemberService.java > a3550b542f04862463a75807859ee7a2a57e2147 > > gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/service/MemberDetailsService.java > 273ebecdb0bb50b4b4f8cd4509a6b460d7cf7dad > > gemfire-pulse/src/main/java/com/vmware/gemfire/tools/pulse/internal/service/MembersListService.java > f51de1669e510a74fae73294bc105a8280b4e381 > gemfire-pulse/src/main/webapp/scripts/pulsescript/clusterDetail.js > 8a6c08a80c04816a1b097127b6e459278c7986e3 > > gemfire-pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseBaseTest.java > 74cfa8dab28249a2491e115f55aeb5cf54f4c6f9 > > gemfire-pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseTest.java > ec0608575ad78953336f2e12b6031d1a71edc81d > > Diff: https://reviews.apache.org/r/43507/diff/ > > > Testing > ------- > > uiTest for pulse > > > Thanks, > > Jinmei Liao > >
