[ https://issues.apache.org/jira/browse/YARN-9996?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17081166#comment-17081166 ]
Siddharth Ahuja edited comment on YARN-9996 at 4/11/20, 9:06 AM: ----------------------------------------------------------------- # Instead of manually extracting out the last child of the queue in the for-loop and then doing the same activity in the while-loop for the trimmed queue, my code makes this neater (and simpler) by pulling in the logic outside of the while-loop into the while-loop itself. This code optimization makes a saving of a call each to lastIndexOf() and substring(). # Further, I have made private methods for calls containing lastIndexOf() and substring() for the different scenarios to provide better context of what these calls are about. This abstracts away the need for the person reviewing the code to have to understand the internal logic of how and what needs to be done to extract out the queue names. # Added logging wherever necessary to provide more context. # No new JUnits added as the changes do not introduce/modify existing use-cases, they only optimize them i.e. changes are cosmetic at the end of the day. Class - org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.conf.TestConfigurationMutationACLPolicies already contains test methods relevant for QueueAdminConfigurationMutationACLPolicy. Re-ran the existing test methods to ensure changes are good. was (Author: sahuja): # Instead of manually extracting out the last child of the queue in the for-loop and then doing the same activity in the while-loop for the trimmed queue, my code makes this neater my pulling in the logic outside of the while-loop into the while-loop itself. This code optimization makes a saving of a call each to lastIndexOf() and substring(). # Further, I have made private methods for calls containing lastIndexOf() and substring() for the different scenarios to provide better context of what these calls are about. This abstracts away the need for the person reviewing the code to have to understand the internal logic of how and what needs to be done to extract out the queue names. # Added logging wherever necessary to provide more context. # No new JUnits added as the changes do not introduce/modify existing use-cases, they only optimize them i.e. changes are cosmetic at the end of the day. Class - org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.conf.TestConfigurationMutationACLPolicies already contains test methods relevant for QueueAdminConfigurationMutationACLPolicy. Re-ran the existing test methods to ensure changes are good. > Code cleanup in QueueAdminConfigurationMutationACLPolicy > -------------------------------------------------------- > > Key: YARN-9996 > URL: https://issues.apache.org/jira/browse/YARN-9996 > Project: Hadoop YARN > Issue Type: Improvement > Reporter: Szilard Nemeth > Assignee: Siddharth Ahuja > Priority: Major > Attachments: YARN-9996.001.patch > > > Method 'isMutationAllowed' contains many uses of substring and lastIndexOf. > These could be extracted and simplified. > Also, some logging could be added as well. -- This message was sent by Atlassian Jira (v8.3.4#803005) --------------------------------------------------------------------- To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org