Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some code design suggestions #1

Open
FluentCoding opened this issue Jan 8, 2021 · 1 comment
Open

Some code design suggestions #1

FluentCoding opened this issue Jan 8, 2021 · 1 comment

Comments

@FluentCoding
Copy link

FluentCoding commented Jan 8, 2021

Small thing I've noticed - you are repeating the code segment that is handling the download of the files a lot. You should generally t ry to abstract these code segments into one method (and make a parameter for each thing that changes inside a code segment).
For example:

    R.id.navigation_vplan1 -> {
            val myWebView: WebView = findViewById(R.id.webview)
            myWebView.settings.javaScriptEnabled = true
            webview.webViewClient = WebViewClient()
            myWebView.loadUrl("https://edu.blista.de/moodle/mod/resource/view.php?id=39")

            webview.setDownloadListener({ url, userAgent, contentDisposition, mimeType, contentLength ->
                val request = DownloadManager.Request(Uri.parse(url))
                request.setMimeType(mimeType)
                request.addRequestHeader("cookie", CookieManager.getInstance().getCookie(url))
                request.addRequestHeader("User-Agent", userAgent)
                request.setDescription("Downloading file...")
                request.setTitle(URLUtil.guessFileName(url, contentDisposition, mimeType))
                request.allowScanningByMediaScanner()
                request.setNotificationVisibility(DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_COMPLETED)
                request.setDestinationInExternalFilesDir(this@MainActivity, Environment.DIRECTORY_DOWNLOADS, ".png")
                val dm = getSystemService(Context.DOWNLOAD_SERVICE) as DownloadManager
                dm.enqueue(request)
                Toast.makeText(applicationContext, "Lade Datei...", Toast.LENGTH_LONG).show()
            })

            myWebView.setWebViewClient(object : WebViewClient() {
                override fun shouldOverrideUrlLoading(view: WebView, url: String?): Boolean {
                    if (url != null && url.startsWith("mailto:")) {
                        view.context.startActivity(
                            Intent(Intent.ACTION_VIEW, Uri.parse(url))
                        )
                        return true
                    } else if (url != null && url.startsWith("tel:")) {
                        view.context.startActivity(
                            Intent(Intent.ACTION_VIEW, Uri.parse(url))
                        )
                        return true
                    } else {
                        return false
                    }
                }
            })

            return@OnNavigationItemSelectedListener true
        }
        R.id.navigation_vplan2 -> {

            val myWebView: WebView = findViewById(R.id.webview)
            myWebView.settings.javaScriptEnabled = true
            webview.webViewClient = WebViewClient()
            myWebView.loadUrl("https://edu.blista.de/moodle/mod/resource/view.php?id=41")

            webview.setDownloadListener({ url, userAgent, contentDisposition, mimeType, contentLength ->
                val request = DownloadManager.Request(Uri.parse(url))
                request.setMimeType(mimeType)
                request.addRequestHeader("cookie", CookieManager.getInstance().getCookie(url))
                request.addRequestHeader("User-Agent", userAgent)
                request.setDescription("Downloading file...")
                request.setTitle(URLUtil.guessFileName(url, contentDisposition, mimeType))
                request.allowScanningByMediaScanner()
                request.setNotificationVisibility(DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_COMPLETED)
                request.setDestinationInExternalFilesDir(this@MainActivity, Environment.DIRECTORY_DOWNLOADS, ".png")
                val dm = getSystemService(Context.DOWNLOAD_SERVICE) as DownloadManager
                dm.enqueue(request)
                Toast.makeText(applicationContext, "Lade Datei...", Toast.LENGTH_LONG).show()
            })

            myWebView.setWebViewClient(object : WebViewClient() {
                override fun shouldOverrideUrlLoading(view: WebView, url: String?): Boolean {
                    if (url != null && url.startsWith("mailto:")) {
                        view.context.startActivity(
                            Intent(Intent.ACTION_VIEW, Uri.parse(url))
                        )
                        return true
                    } else if (url != null && url.startsWith("tel:")) {
                        view.context.startActivity(
                            Intent(Intent.ACTION_VIEW, Uri.parse(url))
                        )
                        return true
                    } else {
                        return false
                    }
                }
            })

            return@OnNavigationItemSelectedListener true
}

You could easily make a method for the lambda you insert into the webview downloadListener because the logic simply the same. You could also make a method that handles the loading of a webview (and make a parameter because the url changes everytime).

There are many other places in the code where you duplicated the code so I would suggest you to skim over your code and find these occurrences. Also, maybe you should change some variable names so that they conform to the naming convention that your project (or all general java projects) use and use a more understandable name: navigation_vplan2 is snake_case which doesn't fit to the other namings in this project. You might want to use navigationVPlan2 instead. Also, you should try to avoid numbers in your variable. In general, vplan2 seems to be a weird name so someone like me who doesn't know what this project aims to do should be able to directly know what role this variable has.

Have a nice day!

@FluentCoding FluentCoding changed the title Some Code Design suggestions Some code design suggestions Jan 8, 2021
@z1tr0t3c
Copy link
Owner

Thank you for your suggestions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants