- Migrate everything to Kotlin
- Improve cache management
- Migrate to Ktor
- Improve error handling (not tested thoroughly)
- Break the API (v1.0 now?)
- Nuke LazyMap
- Allow deleting one's own posts
Details
Details
Unit tests are passing. I haven't run the SpotBugs yet, but it will surely flag some errors, none of which I'll understand
Diff Detail
Diff Detail
- Repository
- rLIBWFJVA WildFyre Java Libary
- Branch
- master
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 714 Build 714: arc lint + arc unit
Event Timeline
| .idea/jarRepositories.xml | ||
|---|---|---|
| 1 ↗ | (On Diff #988) | Why do you need this file? It looks like internal stuff that IDEA can get from Gradle anyway, so shouldn't be necessary |
| src/main/java/net/wildfyre/api/Internal.kt | ||
| 35 | The Dokka link should probably be [init] and not [.init] | |
| 39 | What is this used for? | |
| 42 | All three should probably be lateinit instead of nullable | |
| 48 | Possibly broken Dokka link | |
| 58 | Possibly broken Dokka link | |
| 84 | This method is not necessary anymore thanks to all attributes having their automatically generated getters in Kotlin | |
| 85 | Probably shouldn't throw an NPE. At least use a custom error message to explain that the Internal wasn't initialised correctly | |
| 129 | Possibly broken Dokka link If you run Dokka (there should be a Gradle task), it should report all of these as warnings | |
| 132 | Should be a setter on a token property | |
| src/main/java/net/wildfyre/api/WildFyre.kt | ||
| 26 | Should probably be object | |
| 85 | Should be a property with a custom getter | |
| 85 | Shouldn't throw an NPE | |
| 92 | Should be a property with custom getter | |
| 92 | Shouldn't throw an NPE | |
| 104 | The documentation probably should make it more clear what the difference between clean and clear is | |
| src/main/java/net/wildfyre/areas/Area.kt | ||
| 55–56 | internal is a keyword in Kotlin, you probably shouldn't name a variable like that | |
| 56 | Public field requires documentation | |
| 73–74 | It kinda sucks that accessors can't suspend. Well, I guess there's no choice. | |
| .idea/jarRepositories.xml | ||
|---|---|---|
| 1 ↗ | (On Diff #988) | I probably don't need it. |
| src/main/java/net/wildfyre/api/Internal.kt | ||
| 35 | True. | |
| 42 | No, because they don't have to be initialised every time we call init(). And kotlin won't allow us to check if they are null, if they are lateinit, so we would pointlessly initialise new objects | |
| 48 | Probably | |
| 58 | Probably, also probably was there before idk | |
| 84 | It's just a helper because I prefer () to !!. Can be removed sure | |
| 85 | No, it's an internal error so it should throw an internal exception. | |
| 129 | Ah ok thanks | |
| 132 | Yes ok | |
| src/main/java/net/wildfyre/api/WildFyre.kt | ||
| 26 | No! | |
| 104 | Yeah | |
| src/main/java/net/wildfyre/areas/Area.kt | ||
| 55–56 | Nah, kotlin is clever enough. | |
| 56 | Ugh, documentation!?! | |
| 73–74 | I know. I researched it a lot | |
| src/main/java/net/wildfyre/api/Internal.kt | ||
|---|---|---|
| 42 | ||
| 85 | I disagree. If a user has this problem, they should know why it happened. | |
| src/main/java/net/wildfyre/api/WildFyre.kt | ||
| 26 | Why? | |
| src/main/java/net/wildfyre/areas/Area.kt | ||
| 91 | Function is called spread but returns reputation, smells like a copy paste mistake | |
| 247 | Shouldn't throw an NPE (the next step ignores null values anyway) | |
| 292 | Shouldn't throw NPE (next step ignores null values anyway) | |
| src/main/java/net/wildfyre/areas/Areas.kt | ||
| 60 | Where does .kn come from? | |
| src/main/java/net/wildfyre/descriptors/Descriptor.kt | ||
| 53 | Should be written: return lastUsage?.let {
currentTime - ...
} ?: false | |
| src/main/java/net/wildfyre/http/MultiPartContent.kt | ||
| 48 | You're using \r\n everywhere, make it a const val instead of a magic value | |
| src/main/java/net/wildfyre/http/Request.kt | ||
| 52–54 | Put the default parameter at the end and use @JvmOverloads | |
| src/main/java/net/wildfyre/utils/InvalidJsonException.kt | ||
| 65 | != false is meh | |
| src/test/java/net/wildfyre/areas/AreaTest.kt | ||
| 40 | Seems unnecessary | |
| src/main/java/net/wildfyre/api/Internal.kt | ||
|---|---|---|
| 39 | It means that the individual classes don't have to maintain a CacheManager, they can use a one-liner instead | |
| src/main/java/net/wildfyre/api/WildFyre.kt | ||
| 26 | Singletons are bad. I spent literally hours refactoring them away. What if the client app wants to access another account in the background, to get notifications for an alt account? | |
| src/main/java/net/wildfyre/areas/Areas.kt | ||
| 60 | See KDoc in InvalidJsonException | |
| src/main/java/net/wildfyre/http/MultiPartContent.kt | ||
| 48 | but it is a magic value, and it's never going to change. This is almost a verbatim paste from the ktor docs | |
| src/main/java/net/wildfyre/utils/InvalidJsonException.kt | ||
| 65 | It's suggested by JetBrains as better than checking for null and then checking isNull. It's probably optimised away anyway | |
| src/test/java/net/wildfyre/areas/AreaTest.kt | ||
| 40 | It fails otherwise. connectToTestDB returns a value but JUnit tests cannot (or they won't run) | |