Adding ability to have lint rules for JSONFile, exposing extension in… #49
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ankuagarwal
approved these changes
Jun 9, 2019
PawanHegde
requested changes
Jun 10, 2019
| initializeWrappedObjects(file); | ||
| } | ||
|
|
||
| private void initializeWrappedObjects(File file) throws IOException { | ||
| BufferedReader bufferedReader = new BufferedReader(new FileReader(file)); |
There was a problem hiding this comment.
We can use try-with syntax to ensure that this always gets closed no matter the exception
https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html
| } | ||
|
|
||
| @Override | ||
| public void parseAndReplaceWithWrappers() { } |
There was a problem hiding this comment.
So does it still make sense to have this function in the interface if its implementation isn't necessary to be a WrappedObject?
| throw new RuntimeException("Could not parse either a JSONArray or JSONObject from file"); | ||
| } | ||
| public WrappedObject getChild() { | ||
| return wrappedJsonObject != null ? wrappedJsonObject : wrappedJsonArray; |
There was a problem hiding this comment.
Why even have separate variables for wrappedJSONObject and wrappedJsonArray if at the end we only return a WrappedObject? We can have a single variable called wrappedChild and return that in getChild
|
|
||
| @Override | ||
| public String report(JSONFile jsonFile) { | ||
| return "Expect a JSONArray as child"; |
There was a problem hiding this comment.
This message is misleading. Please change.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
#28
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Exposes JSONFile to the linter, allows users to make lint rules for the file.
Adds file extension to the JSONFile
What is the current behavior? (You can also link to an open issue here)
You can't lint on JSONFile
What is the new behavior (if this is a feature change)?
Lint rules for JSONFile can be created.
File extension exposed
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
getObject is no longer usable but should not have been used outside of library.
Other information: