3
\$\begingroup\$

Context: gitlab link

I know, it's a lot of code but I need your help ladies and gentleman. I'm not sure if I can ask for code review for the whole project so I just ask to review this activity. I would like to add this project to my summary (still looking for first work and I'm still student).

This is how interface looks like:

ZooRater ZooWallActivity.kt interface

Usuń z odwiedzonych stands for remove from visited (text changes to add to visited after clicking this button and reverse)

Atrakcje stands for featured.

Zobacz opinie stands for opinions.

Zostaw opinie stands for add opinion

First button is described above, next two buttons redirect you to another activities and the last button displays alert dialog for your opinion. Every star is separate transparent button and star image underneath. This is how it looks like:

ZooRater ZooWallActivity.kt alertDialog

This is the code

class ZooWallActivity : AppCompatActivity() {


val db = FirebaseFirestore.getInstance()
private lateinit var currentUID: String
private lateinit var zooUID: String
private var flag = false

override fun onCreate(savedInstanceState: Bundle?) {
    super.onCreate(savedInstanceState)
    setContentView(R.layout.activity_zoo_wall)
    zooUID = intent.getStringExtra("ZOO_UID")
    currentUID = FirebaseAuth.getInstance().uid ?: ""
    getZooData(zooUID)
    getImage(zooUID)
    fetchRating(zooUID)
    checkIfVisited(currentUID, zooUID)

    zoowall_opinions.setOnClickListener {
        val intent = Intent(this, OpinionsActivity::class.java)
        intent.putExtra("ZOO_UID", zooUID)
        startActivity(intent)
    }

    zoowall_add.setOnClickListener {
        performAddOrRm(currentUID, zooUID)
    }

    zoowall_featured.setOnClickListener {
        featured(zooUID)
    }

    zoowall_addopinion.setOnClickListener {
        var rating = 0
        var customView = layoutInflater.inflate(R.layout.alertdialog_comment, null)
        val builder = AlertDialog.Builder(this)
            .setView(customView)
        val alertDialog = builder.show()
        val opinionsRef =
            db.collection("zoos").document(zooUID).collection("opinions").document(currentUID)
        opinionsRef.get()
            .addOnSuccessListener { opinion ->
                var temp = opinion.data.toString()
                if (temp != "" && temp != "null" && temp != "[]") {
                    customView.alert_comment.setText(opinion.get("comment").toString())
                    rating = opinion.get("rating").toString().toInt()
                    when (rating) {
                        1 -> {
                            customView.alert_rating1_img.setImageResource(android.R.drawable.btn_star_big_on)
                            customView.alert_rating2_img.setImageResource(android.R.drawable.btn_star_big_off)
                            customView.alert_rating3_img.setImageResource(android.R.drawable.btn_star_big_off)
                            customView.alert_rating4_img.setImageResource(android.R.drawable.btn_star_big_off)
                            customView.alert_rating5_img.setImageResource(android.R.drawable.btn_star_big_off)
                        }
                        2 -> {
                            customView.alert_rating1_img.setImageResource(android.R.drawable.btn_star_big_on)
                            customView.alert_rating2_img.setImageResource(android.R.drawable.btn_star_big_on)
                            customView.alert_rating3_img.setImageResource(android.R.drawable.btn_star_big_off)
                            customView.alert_rating4_img.setImageResource(android.R.drawable.btn_star_big_off)
                            customView.alert_rating5_img.setImageResource(android.R.drawable.btn_star_big_off)
                        }
                        3 -> {
                            customView.alert_rating1_img.setImageResource(android.R.drawable.btn_star_big_on)
                            customView.alert_rating2_img.setImageResource(android.R.drawable.btn_star_big_on)
                            customView.alert_rating3_img.setImageResource(android.R.drawable.btn_star_big_on)
                            customView.alert_rating4_img.setImageResource(android.R.drawable.btn_star_big_off)
                            customView.alert_rating5_img.setImageResource(android.R.drawable.btn_star_big_off)
                        }
                        4 -> {
                            customView.alert_rating1_img.setImageResource(android.R.drawable.btn_star_big_on)
                            customView.alert_rating2_img.setImageResource(android.R.drawable.btn_star_big_on)
                            customView.alert_rating3_img.setImageResource(android.R.drawable.btn_star_big_on)
                            customView.alert_rating4_img.setImageResource(android.R.drawable.btn_star_big_on)
                            customView.alert_rating5_img.setImageResource(android.R.drawable.btn_star_big_off)
                        }
                        5 -> {
                            customView.alert_rating1_img.setImageResource(android.R.drawable.btn_star_big_on)
                            customView.alert_rating2_img.setImageResource(android.R.drawable.btn_star_big_on)
                            customView.alert_rating3_img.setImageResource(android.R.drawable.btn_star_big_on)
                            customView.alert_rating4_img.setImageResource(android.R.drawable.btn_star_big_on)
                            customView.alert_rating5_img.setImageResource(android.R.drawable.btn_star_big_on)
                        }
                        else -> {
                            rating = 0
                            customView.alert_rating1_img.setImageResource(android.R.drawable.btn_star_big_off)
                            customView.alert_rating2_img.setImageResource(android.R.drawable.btn_star_big_off)
                            customView.alert_rating3_img.setImageResource(android.R.drawable.btn_star_big_off)
                            customView.alert_rating4_img.setImageResource(android.R.drawable.btn_star_big_off)
                            customView.alert_rating5_img.setImageResource(android.R.drawable.btn_star_big_off)
                        }
                    }
                }
            }
            .addOnFailureListener {

            }

        customView.alertdialog_cancel.setOnClickListener {
            alertDialog.dismiss()
        }
        customView.alertdialog_ok.setOnClickListener {
            val temp = customView.alert_comment.text.toString()
            if (rating == 0) {
                Toast.makeText(this, "Zapomniałeś o ocenie", Toast.LENGTH_LONG).show()
            } else if (temp == "" || temp == "null") {
                Toast.makeText(this, "Komentarz nie może być pusty", Toast.LENGTH_LONG).show()
            } else {
                alertDialog.dismiss()
                val comment = customView.alert_comment.text.toString()
                addComment(rating, comment, currentUID, zooUID)
            }

        }
        customView.alert_rating1_btn.setOnClickListener {
            rating = 1
            customView.alert_rating1_img.setImageResource(android.R.drawable.btn_star_big_on)
            customView.alert_rating2_img.setImageResource(android.R.drawable.btn_star_big_off)
            customView.alert_rating3_img.setImageResource(android.R.drawable.btn_star_big_off)
            customView.alert_rating4_img.setImageResource(android.R.drawable.btn_star_big_off)
            customView.alert_rating5_img.setImageResource(android.R.drawable.btn_star_big_off)
        }
        customView.alert_rating2_btn.setOnClickListener {
            rating = 2
            customView.alert_rating1_img.setImageResource(android.R.drawable.btn_star_big_on)
            customView.alert_rating2_img.setImageResource(android.R.drawable.btn_star_big_on)
            customView.alert_rating3_img.setImageResource(android.R.drawable.btn_star_big_off)
            customView.alert_rating4_img.setImageResource(android.R.drawable.btn_star_big_off)
            customView.alert_rating5_img.setImageResource(android.R.drawable.btn_star_big_off)
        }
        customView.alert_rating3_btn.setOnClickListener {
            rating = 3
            customView.alert_rating1_img.setImageResource(android.R.drawable.btn_star_big_on)
            customView.alert_rating2_img.setImageResource(android.R.drawable.btn_star_big_on)
            customView.alert_rating3_img.setImageResource(android.R.drawable.btn_star_big_on)
            customView.alert_rating4_img.setImageResource(android.R.drawable.btn_star_big_off)
            customView.alert_rating5_img.setImageResource(android.R.drawable.btn_star_big_off)
        }
        customView.alert_rating4_btn.setOnClickListener {
            rating = 4
            customView.alert_rating1_img.setImageResource(android.R.drawable.btn_star_big_on)
            customView.alert_rating2_img.setImageResource(android.R.drawable.btn_star_big_on)
            customView.alert_rating3_img.setImageResource(android.R.drawable.btn_star_big_on)
            customView.alert_rating4_img.setImageResource(android.R.drawable.btn_star_big_on)
            customView.alert_rating5_img.setImageResource(android.R.drawable.btn_star_big_off)
        }
        customView.alert_rating5_btn.setOnClickListener {
            rating = 5
            customView.alert_rating1_img.setImageResource(android.R.drawable.btn_star_big_on)
            customView.alert_rating2_img.setImageResource(android.R.drawable.btn_star_big_on)
            customView.alert_rating3_img.setImageResource(android.R.drawable.btn_star_big_on)
            customView.alert_rating4_img.setImageResource(android.R.drawable.btn_star_big_on)
            customView.alert_rating5_img.setImageResource(android.R.drawable.btn_star_big_on)
        }
    }
}

private fun fetchRating(zooUID: String) {
    val opinionsRef = db.collection("zoos").document(zooUID).collection("opinions")
    opinionsRef.get()
        .addOnSuccessListener { opinions ->
            var rating = 0.0
            var counter = 0.0
            if (opinions.toString() != "null" && opinions.toString() != "[]") {
                opinions.forEach { opinion ->
                    counter++
                    rating += opinion.get("rating").toString().toInt()
                }
            }
            zoowall_counter.text = "(" + counter.toInt().toString() + ")"
            var average = rating.div(counter)
            when (average) {
                in 1.0..1.5 -> {
                    zoowall_rating1.setImageResource(android.R.drawable.btn_star_big_on)
                    zoowall_rating2.setImageResource(android.R.drawable.btn_star_big_off)
                    zoowall_rating3.setImageResource(android.R.drawable.btn_star_big_off)
                    zoowall_rating4.setImageResource(android.R.drawable.btn_star_big_off)
                    zoowall_rating5.setImageResource(android.R.drawable.btn_star_big_off)
                }
                in 1.5..2.5 -> {
                    zoowall_rating1.setImageResource(android.R.drawable.btn_star_big_on)
                    zoowall_rating2.setImageResource(android.R.drawable.btn_star_big_on)
                    zoowall_rating3.setImageResource(android.R.drawable.btn_star_big_off)
                    zoowall_rating4.setImageResource(android.R.drawable.btn_star_big_off)
                    zoowall_rating5.setImageResource(android.R.drawable.btn_star_big_off)
                }
                in 2.5..3.5 -> {
                    zoowall_rating1.setImageResource(android.R.drawable.btn_star_big_on)
                    zoowall_rating2.setImageResource(android.R.drawable.btn_star_big_on)
                    zoowall_rating3.setImageResource(android.R.drawable.btn_star_big_on)
                    zoowall_rating4.setImageResource(android.R.drawable.btn_star_big_off)
                    zoowall_rating5.setImageResource(android.R.drawable.btn_star_big_off)
                }
                in 3.5..4.5 -> {
                    zoowall_rating1.setImageResource(android.R.drawable.btn_star_big_on)
                    zoowall_rating2.setImageResource(android.R.drawable.btn_star_big_on)
                    zoowall_rating3.setImageResource(android.R.drawable.btn_star_big_on)
                    zoowall_rating4.setImageResource(android.R.drawable.btn_star_big_on)
                    zoowall_rating5.setImageResource(android.R.drawable.btn_star_big_off)
                }
                in 4.5..5.0 -> {
                    zoowall_rating1.setImageResource(android.R.drawable.btn_star_big_on)
                    zoowall_rating2.setImageResource(android.R.drawable.btn_star_big_on)
                    zoowall_rating3.setImageResource(android.R.drawable.btn_star_big_on)
                    zoowall_rating4.setImageResource(android.R.drawable.btn_star_big_on)
                    zoowall_rating5.setImageResource(android.R.drawable.btn_star_big_on)
                }
                else -> {
                }
            }
        }
}

private fun addComment(rating: Int, comment: String, userUID: String, zooUID: String) {
    val opinionsRef = db.collection("zoos").document(zooUID).collection("opinions")
    val opinion = mapOf<String, Any>(
        "comment" to comment,
        "rating" to rating
    )
    opinionsRef.document(currentUID).set(opinion)
        .addOnSuccessListener {
            fetchRating(zooUID)
        }
}

private fun featured(UID: String) {
    val intent = Intent(this, FeaturedActivity::class.java)
    intent.putExtra("ZOO_UID", UID)
    startActivity(intent)
}

private fun performAddOrRm(uid: String, UID: String) {
    zoowall_add.isEnabled = false
    zoowall_add.isClickable = false
    if (flag) {
        db.collection("users").document(uid).update(
            "visited_zoos", FieldValue.arrayRemove(UID)
        )
            .addOnSuccessListener {
                zoowall_add.isEnabled = true
                zoowall_add.isClickable = true
                zoowall_add.text = "Dodaj do odwiedzonych"
                flag = false
            }
            .addOnFailureListener {
                Toast.makeText(this, "${it.message}", Toast.LENGTH_LONG).show()
            }
    } else {
        db.collection("users").document(uid).update(
            "visited_zoos", FieldValue.arrayUnion(UID)
        )
            .addOnSuccessListener {
                zoowall_add.isEnabled = true
                zoowall_add.isClickable = true
                zoowall_add.text = "Usuń z odwiedzonych"
                flag = true
            }
            .addOnFailureListener {
                Toast.makeText(this, "${it.message}", Toast.LENGTH_LONG).show()
            }
    }
}

private fun checkIfVisited(uid: String, UID: String) {
    db.collection("users").document(uid).get()
        .addOnSuccessListener { user ->
            val temp = user.get("visited_zoos").toString()
            if (temp != "" && temp != "[]" && temp != "null") {
                val visitedZoos = user.get("visited_zoos") as List<String>
                visitedZoos.forEach { zoo ->
                    if (zoo == UID) {
                        zoowall_add.text = "Usuń z odwiedzonych"
                        flag = true
                    }
                }
            }
        }
}

private fun getImage(UID: String) {
    val storage = FirebaseStorage.getInstance()
    val storageRef = storage.reference
    db.collection("zoos").document(UID).get()
        .addOnSuccessListener { zoo ->
            val path = zoo.get("photo").toString()
            if (path != "" && path != "null") {
                storageRef.child(path).downloadUrl
                    .addOnSuccessListener {
                        Picasso.get().load(it).into(zoowall_photo)
                    }
                    .addOnFailureListener {
                        zoowall_photo_text.text = "Brak zdjęcia"
                    }
            }
        }
}

private fun getZooData(UID: String) {
    db.collection("zoos").document(UID).get()
        .addOnSuccessListener {
            val name: String = it.get("name").toString()
            val address: String = it.get("address").toString()
            if (name != "" && name != "null") zoowall_name.text = name
            else zoowall_name.text = "Brak nazwy zoo"
            if (address != "" && address != "null") zoowall_address.text = address
            else zoowall_address.text = "Brak adresu zoo"
        }
        .addOnFailureListener {
            Toast.makeText(this, "${it.message}", Toast.LENGTH_LONG).show()
            zoowall_name.text = "Brak nazwy zoo"
            zoowall_address.text = "Brak adresu zoo"
        }
}
\$\endgroup\$
8
  • \$\begingroup\$ "I'm not sure if I can ask for code review for the whole project" Depends, how big is it in character count? We got a fairly high limit, but if you have enough sections the size of the piece you posted it's going to be tricky. Welcome to the site anyway :-) \$\endgroup\$
    – Mast
    Commented Jan 14, 2020 at 20:28
  • \$\begingroup\$ How do you handle your back-end and CRUD actions on the permanent storage? \$\endgroup\$
    – Mast
    Commented Jan 14, 2020 at 20:29
  • \$\begingroup\$ @Mast I thought about adding gitlab repo link - is it permitted on this site or should I just post whole project code here? How I handle CRUD operations - I create database reference and then I use on it get/set/update method on it. val opinionsRef = db.collection("zoos").document(zooUID).collection("opinions").document(currentUID) opinionsRef.get() .addOnSuccessListener { opinion -> \$\endgroup\$ Commented Jan 14, 2020 at 20:34
  • \$\begingroup\$ Is it permitted to post a link to GitHub? Yes. Will the code be reviewed? No. Only the code that's posted in the question is up for review, the rest is context. \$\endgroup\$
    – Mast
    Commented Jan 14, 2020 at 20:35
  • \$\begingroup\$ I recommend you to follow the Android-tutorial created by google. udacity.com/course/developing-android-apps-with-kotlin--ud9012 Followed by: udacity.com/course/advanced-android-with-kotlin--ud940 Optionally, you can first follow a kotlin-course if you want: udacity.com/course/kotlin-bootcamp-for-programmers--ud9011 All the courses are provided by Google themselves and are free. The kotlin course goes through Kotlin reasonable quick and the Android-courses will focus a lot on architecture. \$\endgroup\$
    – tieskedh
    Commented Jan 15, 2020 at 13:25

1 Answer 1

2
\$\begingroup\$

Anko commons

I think Anko is dead, however, their library is still pretty useful.
I would recommend commons, such that you can rewrite the activity-starting

zoowall_opinions.setOnClickListener {
    val intent = Intent(this, OpinionsActivity::class.java)
    intent.putExtra("ZOO_UID", zooUID)
    startActivity(intent)
}

with Anko-commons will become:

zoowall_opinions.setOnClickListener {
    startActivity<OpinionsActivity>("ZOO_UID", zooUID)
}

rating

You should extract setting the buttons in another function:

fun setRating(customView : View, ratingNr: Int){
    val views = listOf(
        customView.alert_rating1_img,
        customView.alert_rating2_img,
        customView.alert_rating3_img,
        customView.alert_rating4_img,
        customView.alert_rating5_img
    )

    //takes ratingNr buttons and turns them on.
    views.take(rating).forEach{ 
          it.setImageResource(android.R.drawable.btn_star_big_on)
    }
    //drops ratingNr buttons and turns the rest of
    views.drop(rating).forEach{
          it.setImageResource(android.R.drawable.btn_star_big_off)
    }
}

BTW, It's called a RatingBar.
You can find multiple libraries on GitHub and you can find the build-in one RatingBar.

fetchRating

Kotlin has operator overloadingin. This means that rating.div(counter) can be rewritten as rating/counter.
Next, if you devide, only one side has to be an double in order to get a double as result. therefor, counter can be an int.

takeIf

Kotlin has a function named takeIf. It will return the receiver if the value is met and otherwise, it will return null.

if (opinions.toString() != "null" && opinions.toString() != "[]") {
    opinions.forEach { opinion ->
        counter++
        rating += opinion.get("rating").toString().toInt()
    }
}

can therefor be rewritten as

opionions
     .takeIf{it.toString() != "null" && it.toString() != "[]" }
     ?.forEach{...}

or using string interprolation

opionions
     .takeIf{"$it" != "null" && "$it" != "[]" }
     ?.forEach{...}

Next, you want to change the opions to a list of Int:

val intOpinions = opinions.map{ it.get("rating").toString().toInt() }

Now you can simply calculate the average, using average and the size using size.

Average is a double and if you RatingBar, you need to have an Int to call our function. Therefor use roundToInt.

checkIfVisited

visitedZoos.forEach { zoo ->
    if (zoo == UID) {
        zoowall_add.text = "Usuń z odwiedzonych"
        flag = true
    }
}

can be rewritten using any:

val isVisited = visited.any { zoo == UID }
if(isVisited) {
    zoowall_add.text = "Usuń z odwiedzonych"
    flag = true
}

coroutines:

using kotlinx-coroutines-play-services, you can replace

ref.get()
    .addOnsucceccListener{ value -> println(value) }
    .addOnFailureListener{ println("failure") }

with

try {
    val value = ref.get().await()
    println(value)
} catch(e : Exception) {
    println("failure")
}

To call coroutine-functions, you have to be in a coroutine-context.
If you want to learn more about coroutine, follow Google's Codelab.
I think (don't know for sure), that they are also explained in the course In the comments of your question. Anko-layouts gives you easy access to coroutine-contexts.

Architecture

I will hint very slightly to the architecture.

Everything should do one thing and one thing only.

  • Your activity should therefor (almost) only deal with how the screen looks.
  • Your firebase-calls should in their callbacks only format the data and give data back.
  • There should be one or more classes that deal with the logic:
    • What should happens if a button is clicked.
    • What data does the view need
    • How to change the data in the format that the view can use

The reason why I push you towards this architecture is that if you learn and stick to a good architecture, it will push you towards classes and functions that do one thing and one thing well. Therefor, your classes and functions will become a lot smaller and also more reuseable. Therefor your code will become better. Almost all you have to do for that is stick to the architecture.

note

I didn't look into firebase functions like queries.

\$\endgroup\$
2
  • \$\begingroup\$ Wow thank you so much <3 as soon as I correct this code I will post it in the comment. Have a good day! \$\endgroup\$ Commented Jan 17, 2020 at 11:19
  • \$\begingroup\$ OK, then I will add an Update-header to my answer and review the new code. \$\endgroup\$
    – tieskedh
    Commented Jan 17, 2020 at 11:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.