Various code style improvements and refactorings

Author: rubengees

config/detekt/detekt.yml

@@ -5,31 +5,27 @@ potential-bugs:
     active: false
 
 complexity:
+  StringLiteralDuplication:
+    threshold: 5
   LongParameterList:
     active: false
   LargeClass:
     active: false
-  ComplexMethod:
-    active: false
   TooManyFunctions:
     active: false
   ComplexCondition:
     active: false
   NestedBlockDepth:
     active: false
-  StringLiteralDuplication:
-    threshold: 5
 
 style:
-  SafeCast:
-    active: false
   FunctionMaxLength:
-    active: false
-  MagicNumber:
-    active: false
+    maximumFunctionNameLength: 35
   VariableMinLength:
     minimumVariableNameLength: 2
   VariableMaxLength:
+    maximumVariableNameLength: 35
+  MagicNumber:
     active: false
   DataClassContainsFunctions:
     active: false

src/main/kotlin/me/proxer/app/auth/LoginDialog.kt

@@ -56,6 +56,17 @@ class LoginDialog : BaseDialog() {
     override fun onActivityCreated(savedInstanceState: Bundle?) {
         super.onActivityCreated(savedInstanceState)
 
+        setupViews()
+        setupViewModels()
+
+        if (savedInstanceState == null) {
+            username.requestFocus()
+
+            dialog.window.setSoftInputMode(WindowManager.LayoutParams.SOFT_INPUT_STATE_VISIBLE)
+        }
+    }
+
+    private fun setupViews() {
         listOf(password, secret).forEach {
             it.editorActionEvents(Predicate { event -> event.actionId() == EditorInfo.IME_ACTION_GO })
                     .filter { event -> event.actionId() == EditorInfo.IME_ACTION_GO }
@@ -71,7 +82,9 @@ class LoginDialog : BaseDialog() {
         }
 
         secret.transformationMethod = null
+    }
 
+    private fun setupViewModels() {
         viewModel.data.observe(this, Observer {
             it?.let {
                 StorageHelper.user = LocalUser(it.loginToken, it.id, username.text.trim().toString(), it.image)
@@ -100,12 +113,6 @@ class LoginDialog : BaseDialog() {
             secret.imeOptions = if (it == true) EditorInfo.IME_ACTION_GO else EditorInfo.IME_ACTION_NEXT
             password.imeOptions = if (it == true) EditorInfo.IME_ACTION_NEXT else EditorInfo.IME_ACTION_GO
         })
-
-        if (savedInstanceState == null) {
-            username.requestFocus()
-
-            dialog.window.setSoftInputMode(WindowManager.LayoutParams.SOFT_INPUT_STATE_VISIBLE)
-        }
     }
 
     private fun validateAndLogin() {

src/main/kotlin/me/proxer/app/chat/sync/ChatJob.kt

@@ -115,66 +115,80 @@ class ChatJob : Job() {
     override fun onRunJob(params: Params): Result {
         if (StorageHelper.user == null) return Result.FAILURE
 
-        val synchronizationResult = conferenceId.let { conferenceId ->
-            when (conferenceId) {
-                0L -> try {
-                    val newConferencesAndMessages = synchronize()
+        val synchronizationResult = when (conferenceId) {
+            0L -> try {
+                handleSynchronization()
+            } catch (error: Throwable) {
+                handleSynchronizationError(error)
+            }
+            else -> try {
+                handleLoadMoreMessages(conferenceId)
+            } catch (error: Throwable) {
+                handleLoadMoreMessagesError(error)
+            }
+        }
 
-                    StorageHelper.areConferencesSynchronized = true
+        reschedule(context, synchronizationResult)
 
-                    if (newConferencesAndMessages.isNotEmpty()) {
-                        bus.post(SynchronizationEvent())
+        return if (synchronizationResult != ERROR) Result.SUCCESS else Result.FAILURE
+    }
 
-                        if (canShowNotification(context)) {
-                            showNotification(context)
-                        }
-                    }
+    private fun handleSynchronization(): SynchronizationResult {
+        val newConferencesAndMessages = synchronize()
 
-                    newConferencesAndMessages.flatMap { it.value }.maxBy { it.date }?.date?.let { mostRecentDate ->
-                        if (mostRecentDate > StorageHelper.lastChatMessageDate) {
-                            StorageHelper.lastChatMessageDate = mostRecentDate
-                        }
-                    }
+        StorageHelper.areConferencesSynchronized = true
 
-                    if (newConferencesAndMessages.isNotEmpty()) CHANGES else NO_CHANGES
-                } catch (error: Throwable) {
-                    when (error) {
-                        is ChatException -> bus.post(ChatErrorEvent(error))
-                        else -> bus.post(ChatErrorEvent(ChatSynchronizationException(error)))
-                    }
+        if (newConferencesAndMessages.isNotEmpty()) {
+            bus.post(SynchronizationEvent())
 
-                    if (ErrorUtils.isIpBlockedError(error)) {
-                        ChatNotifications.showError(context, error)
+            if (canShowNotification(context)) {
+                showNotification(context)
+            }
+        }
 
-                        ERROR
-                    } else {
-                        NO_CHANGES
-                    }
-                }
-                else -> try {
-                    val fetchedMessages = loadMoreMessages(conferenceId)
+        newConferencesAndMessages.flatMap { it.value }.maxBy { it.date }?.date?.let { mostRecentDate ->
+            if (mostRecentDate > StorageHelper.lastChatMessageDate) {
+                StorageHelper.lastChatMessageDate = mostRecentDate
+            }
+        }
 
-                    fetchedMessages.maxBy { it.date }?.date?.let { mostRecentDate ->
-                        if (mostRecentDate > StorageHelper.lastChatMessageDate) {
-                            StorageHelper.lastChatMessageDate = mostRecentDate
-                        }
-                    }
+        return if (newConferencesAndMessages.isNotEmpty()) CHANGES else NO_CHANGES
+    }
 
-                    CHANGES
-                } catch (error: Throwable) {
-                    when (error) {
-                        is ChatException -> bus.post(ChatErrorEvent(error))
-                        else -> bus.post(ChatErrorEvent(ChatMessageException(error)))
-                    }
+    private fun handleSynchronizationError(error: Throwable): SynchronizationResult {
+        when (error) {
+            is ChatException -> bus.post(ChatErrorEvent(error))
+            else -> bus.post(ChatErrorEvent(ChatSynchronizationException(error)))
+        }
 
-                    NO_CHANGES
-                }
-            }
+        return if (ErrorUtils.isIpBlockedError(error)) {
+            ChatNotifications.showError(context, error)
+
+            ERROR
+        } else {
+            NO_CHANGES
         }
+    }
 
-        reschedule(context, synchronizationResult)
+    private fun handleLoadMoreMessagesError(error: Throwable): SynchronizationResult {
+        when (error) {
+            is ChatException -> bus.post(ChatErrorEvent(error))
+            else -> bus.post(ChatErrorEvent(ChatMessageException(error)))
+        }
 
-        return if (synchronizationResult != ERROR) Result.SUCCESS else Result.FAILURE
+        return NO_CHANGES
+    }
+
+    private fun handleLoadMoreMessages(conferenceId: Long): SynchronizationResult {
+        val fetchedMessages = loadMoreMessages(conferenceId)
+
+        fetchedMessages.maxBy { it.date }?.date?.let { mostRecentDate ->
+            if (mostRecentDate > StorageHelper.lastChatMessageDate) {
+                StorageHelper.lastChatMessageDate = mostRecentDate
+            }
+        }
+
+        return CHANGES
     }
 
     private fun synchronize(): Map<LocalConference, List<LocalMessage>> {
@@ -231,7 +245,6 @@ class ChatJob : Job() {
 
             // Per documentation: The api may return some String in case something went wrong.
             if (result != null) {
-
                 // Delete all messages we have correctly sent already.
                 chatDatabase.runInTransaction {
                     for (i in 0..index) {
@@ -279,12 +292,12 @@ class ChatJob : Job() {
         val mostRecentMessage = chatDao.findMostRecentMessageForConference(conference.id.toLong())?.toNonLocalMessage()
 
         return when (mostRecentMessage) {
-            null -> fetchNewMessagesForEmptyConference(conference)
-            else -> fetchNewMessagesForExistingConference(conference, mostRecentMessage)
+            null -> fetchForEmptyConference(conference)
+            else -> fetchForExistingConference(conference, mostRecentMessage)
         }
     }
 
-    private fun fetchNewMessagesForEmptyConference(conference: Conference): Pair<List<Message>, Boolean> {
+    private fun fetchForEmptyConference(conference: Conference): Pair<List<Message>, Boolean> {
         val newMessages = mutableListOf<Message>()
 
         var unreadAmount = 0
@@ -311,7 +324,7 @@ class ChatJob : Job() {
         return newMessages to false
     }
 
-    private fun fetchNewMessagesForExistingConference(conference: Conference, mostRecentMessage: Message):
+    private fun fetchForExistingConference(conference: Conference, mostRecentMessage: Message):
             Pair<List<Message>, Boolean> {
         val mostRecentMessageIdBeforeUpdate = mostRecentMessage.id.toLong()
         val newMessages = mutableListOf<Message>()