Feature/cleanup refactor improve #7
Merged
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
- Updates gradle tools version to 4.0.1 - Update gradle wrapper version to 6.1.1 - Update target and compile sdk version to 30 - Update libraries to the latest - Fix compilatin issue for `getString()`
- Migrates the code from AppCompat to AndroidX - Cleans up gradle files to be more meaningful and readable **Note:** 1. It is recommended to use AndroidX instead of AppCompat after Android 28. Migration was done using the Android Studio's in-built migration tool. However care is taken to make sure nothing is broken after the migration. 2. Remove unnecessary white spaces, empty lines is the first step of the clean up and the libraries listed are organized in the order of 1. Standard Compilation libraries, 2. Android Libraries, 3. 3rd Party libraries(None listed) 4. Test & Android Test Support libraries
- Addresses Kotlin lint warnings - Remove invalid attribute in LinearLayout in the sample code - Cleans up unnecessary try-catch blocks and wraps the entire reading attributes into single try-catch block - Applies default values that avoids crashing & preserves the logic - Adds Kotlin Core KTX library that adds few usable extention functions **Note:** 1. My understandig fo the additional try..catch blocks is to provide flexiblity for the library user to skip out few attributes and avoid crashing the app. While the intention is good, the recommended practice is to let the library user to know what attributes are mandatory, for ex., *text* to be displayed. This can be easily achieved using the `getStringOrThrow()` extention function from Kotlin KTX library. Similiar functions are available for other attributes as well. 2. In order to avoid crashes due to missing declaration of attributes by the library user, default values can be used 3. The idea of letting the user use either **colorScheme** or **colors** is a great idea. The flexibility can be achieved by the combination of default value for colors and the order of checking for **colors** attribute first before checking for **colorScheme**. - If the user declares **colors** not **colorScheme**, the user declared color is set, the color scheme is 0 and ignored. - If the user declares **colorScheme** not **colors**, the colors is set to default (to avoid crash & additional try-catch block), and overridden by the color scheme logic. 4. Catching the root class exception is not recommended. Considering this is a library, the recommended way is to throw appropriate exceptions to inform the library user to avoid declaring the attributes.
ha-yi
approved these changes
Sep 28, 2020
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.
Please refer the commit messages for more detailed information of the changes.