Skip to content

Wyrther code review1 #25

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.

Sign up for GitHub

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

Open
wrytherUC wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Wyrther code review1 #25

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1198992
Renamed query to better describe its function
wrytherUC Mar 5, 2023
be5579e
Corrected spelling, replaced top comments with Kdoc
wrytherUC Mar 6, 2023
7bb7a80
Replaced top comments with Kdoc
wrytherUC Mar 6, 2023
f673eae
Changed how room-compiler was being called by kapt
wrytherUC Mar 6, 2023
c9a6a62
Revert "Changed how room-compiler was being called by kapt"
wrytherUC Mar 6, 2023
3196b6d
Added version var for room database dependencies
wrytherUC Mar 6, 2023
e3129d2
Added non null data
wrytherUC Mar 6, 2023
b77bdf1
Swtiched blank kdoc to the top and put comment back above companion
wrytherUC Mar 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions app/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ dependencies {
debugImplementation "androidx.compose.ui:ui-test-manifest:$compose_ui_version"

// Add Room Database Dependencies
implementation "androidx.room:room-runtime:2.5.0"
kapt "androidx.room:room-compiler:2.5.0"
implementation "androidx.room:room-ktx:2.5.0"
def room_version = "2.5.0"
implementation "androidx.room:room-runtime:$room_version"
Copy link
Copy Markdown
Collaborator Author

@wrytherUC wrytherUC Mar 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added version variable to make it a little easier to update any room dependencies.

Copy link
Copy Markdown

@discospiff discospiff Mar 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. Dependencies are like whack a mole in Android.

kapt "androidx.room:room-compiler:$room_version"
implementation "androidx.room:room-ktx:$room_version"
}
2 changes: 1 addition & 1 deletion app/src/main/java/com/medmapper/v33001/dao/MedicineDAO.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import kotlinx.coroutines.flow.Flow
@Dao
interface MedicineDAO {
@Query("SELECT * FROM Medicine ORDER BY name ASC")
fun getAlphabetizedMedicine(): Flow<List<Medicine>>
fun getAllAlphabetizedMedicine(): Flow<List<Medicine>>
Copy link
Copy Markdown
Collaborator Author

@wrytherUC wrytherUC Mar 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just added All to the name of the function, as the query is fetching all rows from the Medicine DB


@Insert(onConflict = OnConflictStrategy.IGNORE)
suspend fun insert(medicine: Medicine)
Expand Down
7 changes: 4 additions & 3 deletions app/src/main/java/com/medmapper/v33001/dto/Medicine.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
package com.medmapper.v33001.dto

import androidx.annotation.NonNull
import androidx.room.ColumnInfo
import androidx.room.Entity
import androidx.room.PrimaryKey

@Entity(tableName = "Medicine")
data class Medicine(
@PrimaryKey(autoGenerate = true) val medID: Int,
@ColumnInfo(name = "name") val name: String?,
@NonNull @ColumnInfo(name = "name") val name: String,
@ColumnInfo(name = "strength") val strength: String?,
@ColumnInfo(name = "start date") val startDate: String?,
@ColumnInfo(name = "prescription length") val lengthInDays: Int
@NonNull @ColumnInfo(name = "start date") val startDate: String,
@NonNull @ColumnInfo(name = "prescription length") val lengthInDays: Int
//,@ColumnInfo() val endDate: Date = startDate
Copy link
Copy Markdown
Collaborator Author

@wrytherUC wrytherUC Mar 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added non nulls, as the name of the medication, start date, and start day should be required values.

)
20 changes: 12 additions & 8 deletions app/src/main/java/com/medmapper/v33001/repository/MedicineRepository.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,21 @@ import com.medmapper.v33001.dao.MedicineDAO
import com.medmapper.v33001.dto.Medicine
import kotlinx.coroutines.flow.Flow

// Declares teh DAO as private property in the constructor. Pass in the DAO
// instead of the whole database, because you only need access to the DAO
/**
* Declares the DAO as private property in the constructor.
* Pass in the DAO instead of the whole database, because you only need access to the DAO
*/
Copy link
Copy Markdown
Collaborator Author

@wrytherUC wrytherUC Mar 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added kdoc above the class

Copy link
Copy Markdown

@discospiff discospiff Mar 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good.

class MedicineRepository(private val medicineDAO: MedicineDAO) {

// Room executes all queries on a separate thread.
// Observed Flow will notify the observer when the data has changed.
val allMedicines: Flow<List<Medicine>> = medicineDAO.getAlphabetizedMedicine()
// Room executes all queries on a separate thread.
// Observed Flow will notify the observer when the data has changed.

val allMedicines: Flow<List<Medicine>> = medicineDAO.getAllAlphabetizedMedicine()

// By default Room runs suspend queries off the main thread, therefore
// we don't need to implement anything else to ensure we're not doing
// long running database work off the main thread

// By default Room runs suspend queries off the main thread, therefore
// we don't need to implement anything else to ensure we're not doing
// long running database work off the main thread
@Suppress("RedundantSuspendModifier")
@WorkerThread
suspend fun insert(medicine: Medicine) {
Expand Down
5 changes: 4 additions & 1 deletion app/src/main/java/com/medmapper/v33001/room/MedicineRoomDatabase.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,16 @@ import androidx.room.RoomDatabase
import com.medmapper.v33001.dao.MedicineDAO
import com.medmapper.v33001.dto.Medicine

/**
*
*/
Comment on lines +10 to +12
Copy link
Copy Markdown

@discospiff discospiff Mar 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty KDoc does not add value.

@Database(entities = arrayOf(Medicine::class), version = 1, exportSchema = false)
public abstract class MedicineRoomDatabase : RoomDatabase() {

abstract fun medicineDao(): MedicineDAO

//Singleton prevents multiple instances of database opening at the same time.
companion object {
Copy link
Copy Markdown
Collaborator Author

@wrytherUC wrytherUC Mar 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved comment out of the actual code block and added a blank kdoc section for future documentation. Since it is a public class, there should be some documentation for it.
https://kotlinlang.org/docs/coding-conventions.html#coding-conventions-for-libraries

// Singleton prevents multiple instances of database opening at the same time.
@Volatile
private var INSTANCE: MedicineRoomDatabase? = null
fun getDatabase(context: Context): MedicineRoomDatabase {
Expand Down