Feature/impl db #105
Feature/impl db #105
Conversation
| @@ -22,55 +24,55 @@ class AboutActivity : AppCompatActivity() { | |||
| override fun onCreate(savedInstanceState: Bundle?) { | |||
| super.onCreate(savedInstanceState) | |||
| setContentView(R.layout.activity_about) | |||
| val toolbar = findViewById(R.id.toolbar) as Toolbar | |||
| val toolbar = findViewById<Toolbar>(R.id.toolbar) | |||
There was a problem hiding this comment.
It's generally a lot cleaner to use generic inference rather than explicitly defining the generic and relying on type inference such as
val toolbar: Toolbar = findViewById(R.id.toolbar)In at least one use case you ended up specifying it both ways.
| @@ -22,55 +24,55 @@ class AboutActivity : AppCompatActivity() { | |||
| override fun onCreate(savedInstanceState: Bundle?) { | |||
| super.onCreate(savedInstanceState) | |||
| setContentView(R.layout.activity_about) | |||
| val toolbar = findViewById(R.id.toolbar) as Toolbar | |||
| val toolbar = findViewById<Toolbar>(R.id.toolbar) | |||
| setSupportActionBar(toolbar) | |||
| supportActionBar?.setDisplayHomeAsUpEnabled(true) | |||
| bind() | |||
| } | |||
|
|
|||
| fun bind() { | |||
There was a problem hiding this comment.
Consider using getters instead of a large bind method.
private val logo: ImageView
get() = findViewById(R.id.cfo_logo)| .autoConnect() | ||
| } | ||
|
|
||
| private fun toAnimalList(response: PetfinderPetRecordResponse): List<Animal> { |
There was a problem hiding this comment.
A more functional approach would be this.
private fun toAnimalList(response: PetfinderPetRecordResponse): List<Animal>
= response.petfinder
.pets
.petList
.map { animal ->
Animal.AnimalBuilder()
...
.createAnimal()
}| return outputAnimals | ||
| } | ||
|
|
||
| private fun photosToStringUrls(photos: PetfinderAnimal.Media.Photos?): List<String> { |
There was a problem hiding this comment.
Consider the previously mentioned functional approach
| return urlList | ||
| } | ||
|
|
||
| private fun getBreed(breed: PetfinderAnimal.Breeds.Breed?): String { |
There was a problem hiding this comment.
This can be consolidated to a single line
return bread?.let{ breed.content } ?: ""This is the logical equivalent of
if (breed != null) {
return breed.content;
}
return "";|
|
||
| class AnimalMapper @Inject constructor(){ | ||
|
|
||
| fun map(from: Animal): AnimalEntity { |
There was a problem hiding this comment.
I would strongly suggest avoiding the function name of map in kotlin as it will be confusing to a reader. A more specific name would remove any confusion. mapToAnimalEntity
| import javax.inject.Inject | ||
|
|
||
|
|
||
| class AnimalMapper @Inject constructor(){ |
There was a problem hiding this comment.
This seems to be a utility class which means it should not be a class in kotlin. Instead of placeholder classes purely to provide extension functionality just write extension functions and get rid of the container class.
fun Animal.mapToAnimalEntity(): AnimalEntity
...
Room Database implemented. We are using two tables, Animals and Images. Through the
AnimalAndImageDao, you have all the properties to map it toAnimal.Changes:
Converted PetfinderProvider to Kotlin
Updated support library to 27.1
Updated SDK to 26 while making roomdb compatible with project.