Skip to content

refactored code#107

Open
bshivangi47 wants to merge 1 commit intoRefactoring-Bot:masterfrom
bshivangi47:master
Open

refactored code#107
bshivangi47 wants to merge 1 commit intoRefactoring-Bot:masterfrom
bshivangi47:master

Conversation

@bshivangi47
Copy link

Rename Variable
The variable json did not signify what the json was for and hence, it was renamed.
Location: Line 252 in GitlabDataGrabber.java

Extract Method
The code to get a URI was repeated in most of the methods in class GithubDataGrabber.java and hence, the method getUri() was extracted
Location: GithubDataGrabber.java

Extract Class
The class ApiGrabber was performing two responsibilities: managing repositories and managing analysis services. Hence, the APIs for Analysis were extracted in a separate class
Location: ApiAnalysisGrabber.java

Move method
Moved method getFieldDeclarationByLineNumber() from RefactoringHelper.java to RefactoringModifier.java as it was only used once in the class RefactoringModifier.java

Replace conditional with polymorphism
All the methods contained switch cases for each ApiGrabber, hence created an interface named ApiGrabberInterface
Location: ApiGrabberInterface.java

@MarvinWyrich
Copy link
Member

Hey,

Thanks a lot for taking the time to refactor several code sections and describing your changes so clearly. Looks very good at first glance. I'll take a closer look at the changes as soon as possible.

Best regards,
Marvin

@bshivangi47
Copy link
Author

Thank you so much for your kind words. This refactoring was a part of my course assignment and it will be really beneficial for my grades if my pull request gets accepted by the end of this week (deadline for assignment) as it will suggest that I have done great job in this assignment. I really hope that everything is correct and my request gets accepted.

Thank you so much,
Shivangi Bhatt

@MarvinWyrich
Copy link
Member

  • Rename Variable refactoring is fine.
  • Extract Method refactoring is fine.
  • Extract Class + Replace conditional with polymorphism: in gitlabApiGrabber there are a couple of attributes and imports left which are related to GitHub. Please check which of these can be removed.
  • The move method refactoring should be reverted, since the method getFieldDeclarationByLineNumber does not belong to the ReorderModifier class, in particular not as a public static method.
  • Please make sure to follow the few code style rules we have in this project. In particular, please use tabs for indentation for consistency.

Apart from that, it looks good to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants