First delivery #42

TheMaster1802 · 2023-02-02T23:30:54Z

First delivery with the set up, navigation, retrofit adapter and dependency injection.

- Uses permission enabled Gradle - Adding Dependencies MainActivity - Setting up content container

- Adding Screens and Navigation MainActivity - Setting up navHost

- Adding Data Transfer Object (DTO) Interface for Retrofit - Adding AppModule and its provider for RetrofitInstance - Adding Models for Available Book, Ticker and Order Book - Adding New Class which extends from Application for Hilt annotation HiltAndroidApp - Name registration in Manifest MainActivity - Adding entry point Hilt annotation

- Adding Repository provider in AppModule - Creating a Generic data class for each api response Presentation - Creating the repository - Creating ViewModel and its corresponding functions - Implementing ViewModel in Navigation and Principal Screen

- Generic Card component - New SVG Image for app Icon and PlaceHolder - Building up the Principal screen

- Setting up the Navigation flow in order to pass the book name as argument to Description Screen

- Creating an Interface for local and remote repository

em4n0101

Hola Samuel!

Quisiera felicitarte porque me parece muy bueno tu primer entregable! implementaste features que no eran requisitos para este entregable como inyeccion de dependencias y Compose, lo cual hizo tu código mucho mas legible. Te deje algunos comentarios que podrian hacer tu código un poco mas limpio como extraer valores constantes al archivo de dimens y en la parte de compose podrias pasar solo los datos que vas a mostrar por medio del constructor en lugar de todo el viewModel, asi la logica queda mejor encapsulada y tu composable solo se encarga de la parte visual. Tambien la pantalla de detalles la podrias mejorar ya que solo se muestra la respuesta de los servicios en forma de texto.

Sigue asi, por los avances que me has mostrado del siguiente entregable estoy seguro que sera igual de bueno.
Saludos!!

em4n0101 · 2023-02-14T17:07:33Z

app/src/main/java/com/example/baz_android_capstone/components/genericCard/GenericCard.kt

+                .padding(16.dp),
+            horizontalArrangement = Arrangement.Start,
+            verticalAlignment = Alignment.CenterVertically
+        ) {
+            GlideImage(
+                modifier = Modifier
+                    .padding(16.dp)
+                    .clip(shape = CircleShape)
+                    .size(40.dp)


Trata de separar estos valores constantes en un archivo de dimens.

em4n0101 · 2023-02-14T17:09:47Z

...rc/main/java/com/example/baz_android_capstone/components/genericCard/GenericCardInterface.kt

+interface GenericCardInterface {
+    val glideModel: GlideModel
+    val background: Color
+    val title: String?
+    val subtitle: String?
+    val helper: String?
+}
+
+data class GenericCardPresentation(
+    override val glideModel: GlideModel,
+    override val background: Color,
+    override val title: String?,
+    override val subtitle: String? = null,
+    override val helper: String? = null,
+    val onClick: () -> Unit = {}
+) : GenericCardInterface
+
+data class GenericCardDescription(
+    val border: Color? = null,
+    override val glideModel: GlideModel,
+    override val background: Color,
+    override val title: String,
+    override val subtitle: String?,
+    override val helper: String?
+) : GenericCardInterface


Muy buena forma de extraer la lógica en una interfaz principal!

em4n0101 · 2023-02-14T17:11:39Z

app/src/main/java/com/example/baz_android_capstone/components/glideOptimized/GlideOptimized.kt

+                .padding(16.dp)
+                .clip(shape = RoundedCornerShape(size = 14.dp))
+                .size(100.dp)


Igual que el anterior, pasa estos valores a un archivo de dimens

em4n0101 · 2023-02-14T17:11:50Z

app/src/main/java/com/example/baz_android_capstone/components/genericCard/GenericCardList.kt

+    ) {
+        items(elements) { element ->
+            GenericCard(genericCardInterface = element)
+            Spacer(modifier = Modifier.height(16.dp))


Igual que el anterior, pasa estos valores a un archivo de dimens

em4n0101 · 2023-02-14T17:14:19Z

app/src/main/java/com/example/baz_android_capstone/data/models/ticker/RollingAverageChange.kt

+package com.example.baz_android_capstone.data.models.ticker
+
+data class RollingAverageChange(
+    val `6`: String


Seria mejor cambiar el nombre de la variable a una mas legible, puedes usar la notación @SerializedName para esto

em4n0101 · 2023-02-14T17:16:22Z

app/src/main/java/com/example/baz_android_capstone/di/AppModule.kt

+@Module
+@InstallIn(SingletonComponent::class)
+object AppModule {
+    @Singleton
+    @Provides
+    fun provideBookRepository(api: BookAPI) : BookRepositoryInterface = BookRepository(api)
+
+    @Singleton
+    @Provides
+    fun provideBookApi(): BookAPI {
+        return Retrofit.Builder()
+            .baseUrl(Constants.AVAILABLE_BOOK_URL)
+            .addConverterFactory(GsonConverterFactory.create())
+            .build()
+            .create(BookAPI::class.java)
+    }
+}


Muy buena forma de usar inyección de dependencias, aun cuando no era requerimiento para este entregable lo implementaste correctamente!

em4n0101 · 2023-02-14T17:16:59Z

app/src/main/java/com/example/baz_android_capstone/presentation/navigation/Navigation.kt

+            slideInHorizontally(
+                initialOffsetX = { 500 },
+                animationSpec = tween(1300)
+            ) +
+                fadeIn(
+                    initialAlpha = 0.0f,
+                    animationSpec = tween(1300)


Igual pasar estos valores constantes a un archivo dimens

em4n0101 · 2023-02-14T17:26:11Z

app/src/main/java/com/example/baz_android_capstone/presentation/screens/Principal.kt

+@Composable
+fun Principal(
+    navController: NavController,
+    viewModel: BookViewModel


Podrías optimizar esta parte pasando al constructor solo los datos que se van a usar en el composable en lugar de pasar el viewModel, de esa forma tu código en compose solo se encargar de la vista y la lógica la dejas aparte

em4n0101 · 2023-02-14T17:27:18Z

app/src/main/java/com/example/baz_android_capstone/presentation/screens/Splash.kt

+    }
+    Box(contentAlignment = Alignment.Center) {
+        Text(
+            text = "Aquí va un\nSplash Screen",


Me parece que esto solo es de forma temporal, de otro forma pasa estos al archivo de strings

em4n0101 · 2023-02-14T17:33:06Z

app/src/main/java/com/example/baz_android_capstone/presentation/screens/Principal.kt

+            GenericCardPresentation(
+                background = Color.LightGray,
+                title = it.book,
+                glideModel = GlideModel(
+                    url = "https://cryptoicons.org/api/icon/${it.book.substring(startIndex = 0, endIndex = it.book.length - 4)}/200",
+                    isRoundedShape = true,
+                    contentScale = ContentScale.Crop
+                )
+            ) {


Si estas usando las imágenes de esta api seria bueno que checaras el caso en que no se encuentre imagen de la moneda ya que al correr la app son varios los casos en que esto pasa y solo se ve la imagen genérica

HSPC and others added 8 commits January 31, 2023 11:07

First commit

376ccb7

AndroidManifest

e1f7004

- Uses permission enabled Gradle - Adding Dependencies MainActivity - Setting up content container

Presentation

bf4bd31

- Adding Screens and Navigation MainActivity - Setting up navHost

Data

a10639a

- Adding Repository provider in AppModule - Creating a Generic data class for each api response Presentation - Creating the repository - Creating ViewModel and its corresponding functions - Implementing ViewModel in Navigation and Principal Screen

Presentation

4f07d9d

- Generic Card component - New SVG Image for app Icon and PlaceHolder - Building up the Principal screen

Presentation

b288e96

- Setting up the Navigation flow in order to pass the book name as argument to Description Screen

Data

91ea8e1

- Creating an Interface for local and remote repository

em4n0101 reviewed Feb 14, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

First delivery #42

First delivery #42

TheMaster1802 commented Feb 2, 2023

em4n0101 left a comment

em4n0101 Feb 14, 2023

em4n0101 Feb 14, 2023

em4n0101 Feb 14, 2023

em4n0101 Feb 14, 2023

em4n0101 Feb 14, 2023

em4n0101 Feb 14, 2023

em4n0101 Feb 14, 2023

em4n0101 Feb 14, 2023

em4n0101 Feb 14, 2023

em4n0101 Feb 14, 2023

First delivery #42

Are you sure you want to change the base?

First delivery #42

Conversation

TheMaster1802 commented Feb 2, 2023

em4n0101 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment