Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Checklist
rake test:unit
)I have used brain tree in the past for the company I was working with and I am really interested in contributing to this repo. To get myself better with the code base and also to learn more about refactoring techniques, I made some small refactoring changes, which I believe might improve the code readability and management. It was huge learning for me to go through this repo 😁.
I am adding all the details of the refactoring done by and small description of why I think this refactoring is required. Please let me know if you think something is wrong or need more changes, I will do the changes, it will help me get better with the code base.
Refactorings:
Extract Method: (https://refactoring.guru/extract-method)
a. Location
i. Package: src/main/java/com/braintreegateway
ii. Class: Customer.java
iii. Method: Customer(NodeWrapper node)
iv. Link of files of previous commit (before refactoring): https://github.com/braintree/braintree_java/blob/5927f1f212eb41d5aabf2b5889f882042573ad8f/src/main/java/com/braintreegateway/Customer.java
v. Link of file of the commit with refactoring changes (after refactoring): https://github.com/braintree/braintree_java/blob/7c55b6e1f5cda5402387c3475acd8464de147bda/src/main/java/com/braintreegateway/Customer.java
vi. Link to commit: 7c55b6e
b. Description: The contents of Customer was too big and was doing two different things: assigning node variables and creating lists of nodeWrapper. These two things can be divided into two different methods to perform “Extract method refactoring”. I’ve extracted the code into two methods: setCustomerNodeWrapperStrings(node); and setCustomerNodeWrapperResponses(node).
Decompose Conditional: (https://refactoring.guru/decompose-conditional)
a. Location:
i. Package: src/main/java/com/braintreegateway/util/Crypto.java
ii. Class: Crypto.java
iii. Method: secureCompare(String left, String right)
iv. Link of files of previous commit (before refactoring): https://github.com/braintree/braintree_java/blob/449703fa7f1a54067e4a3a93cd9b5adba67bf694/src/main/java/com/braintreegateway/util/Crypto.java
v. Link of file of the commit with refactoring changes (after refactoring): https://github.com/braintree/braintree_java/blob/5927f1f212eb41d5aabf2b5889f882042573ad8f/src/main/java/com/braintreegateway/util/Crypto.java
vi. Link to commit: 5927f1f
b. Description: The method secureCompare had a complex conditional statement. To make it simple I transferred the condition to another method called: checkSecureCompareDirections(), which just returns a Boolean value hence performing Decompose conditional refactoring.
Introduce Explaining Variable: (https://blog.thepete.net/blog/2021/06/24/explaining-variable/#:~:text=An%20Explaining%20Variable%20is%20a,a%20little%20more%20self%2Ddocumenting.)
a. Location:
i. Package: src/main/java/com/braintreegateway
ii. Class: MerchantAccountGateway.java
iii. Method: update(String id, MerchantAccountRequest request)
iv. Link of files of previous commit (before refactoring): [https://github.com/braintree/braintree_java/blob/7c55b6e1f5cda5402387c3475acd8464de147bda/src/main/java/com/braintreegateway/MerchantAccountGateway.java](https://github.com/braintree/braintree_java/blob/7c55b6e1f5cda5402387c3475acd8464de147bda/src/main/java/com/braintreegateway/MerchantAccountGateway.java/)
v. Link of file of the commit with refactoring changes (after refactoring): https://github.com/braintree/braintree_java/blob/741e351ff48b8af9ff3fa068351e0206b594be07/src/main/java/com/braintreegateway/MerchantAccountGateway.java
vi. Link to commit: https://github.com/braintree/braintree_java/commit/741e351ff48b8af9ff3fa068351e0206b594be07
b. Description: The method update makes a put API call using http. For creating the url to make http call there it was directly writing url in the put method. I have made an explaining variable: updateUrl, and uses in put method, instead of directly adding string, now its clear what is the purpose to that String, Hence performing Explaining variable refactoring.
Move method: (https://refactoring.guru/move-method)
a. Location:
i. Package: src/main/java/com/braintreegateway
b. Description: The method connectUrl of clas oAuthGateway.java was mostly just using the objects of Configuration class. So it made more sense to just add it in the Configuration.java class. Since the method was moved from one class to another ( oAuthGateway -> Configuration.java) this is a move method refactoring.
Push-down method: (https://refactoring.guru/push-down-method)
a. Location:
i. Package: src/main/java/com/braintreegateway
b. Description: Request is extended by multiple classes but the method buildXMLElement is being used by just one child class which is: SearchCriteria.java, so instead of having the method in parent we can just push it down to child class, hence making it push down refactoring. The method moved from Request.java (parent) -> SearchCriteria.java (child).
Pull-up method: (https://refactoring.guru/pull-up-method)
a. Location:
i. Package: src/main/java/com/braintreegateway/util
b. Description: There are two methods in classes (Sha1Hasher and Sha256Hasher) hmacHash and sha1Bytes, sha256Bytes, also these two classes implement interface: Hasher. The code in these methods is almost same, it just hardcodes the value of sha algorithm in each individual class. I made changes to pull-up these methods to a parent class called ShaTypeHasher and implement these two methods in this parent class, the SHA algorithm value will be passed as a parameter to the methods. Hence performing pull-up refactoring, since the methods moved from child classes (Sha1Hasher, Sha256Hasher) -> parent (ShaTypeHasher) class. It also resulted in removing duplicate code and making it more readable.