[
https://issues.apache.org/jira/browse/YARN-8553?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16619232#comment-16619232
]
Antal Bálint Steinbach edited comment on YARN-8553 at 9/18/18 2:59 PM:
-----------------------------------------------------------------------
Hi [~snemeth] ,
Thanks for creating the patch. Your changes cleaning the code for sure.
I found some minor things to consider:
1) I would set the boolean fields default value in
_ApplicationsRequestBuilderCommons_ to false. It is more straightforward in
that way.
{code:java}
private boolean startedTimeSpecified;
private boolean finishedTimeSpecified;
private boolean appStatesSpecified;
private boolean appTypesSpecified;{code}
2) in _ApplicationsRequestBuilderCommons_
{code:java}
Set<String> appStates = ApplicationsRequestValueParser
.parseApplicationStates(this.appStates);
{code}
appStates is already a field in the class.
3) _TestApplicationsRequestBuilderCommons_ it would be easier to maintain and
read if you don't check all the "builder setters" one by one. In my opinion, it
is enough to keep the one which tests all of them together, plus the ones which
are testing edge cases or invalid values.
was (Author: bsteinbach):
Hi [~snemeth] ,
Thanks for creating the patch. Your changes cleaning the code for sure.
I found some minor things to consider:
1) I would set the boolean fields default value in
_ApplicationsRequestBuilderCommons_ to false. It is more straightforward in
that way.
{code:java}
private boolean startedTimeSpecified;
private boolean finishedTimeSpecified;
private boolean appStatesSpecified;
private boolean appTypesSpecified;{code}
2) in _ApplicationsRequestBuilderCommons_
{code:java}
// Set<String> appStates = ApplicationsRequestValueParser
.parseApplicationStates(this.appStates);
{code}
appStates is already a field in the class.
3) _TestApplicationsRequestBuilderCommons_ it would be easier to maintain and
read if you don't check all the "builder setters" one by one. In my opinion, it
is enough to keep the one which tests all of them together, plus the ones which
are testing edge cases or invalid values.
> Reduce complexity of AHSWebService getApps method
> -------------------------------------------------
>
> Key: YARN-8553
> URL: https://issues.apache.org/jira/browse/YARN-8553
> Project: Hadoop YARN
> Issue Type: Bug
> Reporter: Rohith Sharma K S
> Assignee: Szilard Nemeth
> Priority: Major
> Attachments: YARN-8553.001.patch, YARN-8553.001.patch,
> YARN-8553.002.patch
>
>
> YARN-8501 refactor the RMWebService#getApp. Similar refactoring required in
> AHSWebservice.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
