Some points on a naming convention and redundant code check

[hoangnguyen92dn]

• It's good to use a pattern to convert the object to another kind for usage, I see you are using SurveyAdapter and TokenAdapter to convert the object to the entity by mapping the object's properties, so I think it should be mapper class instead of adapter. How do you think?
• Following this article

The repository is a mechanism for encapsulating storage, retrieval, and search behavior, which emulates a collection of objects

It means that we could use the repository class in case working with the data sources. With the DispatcherRepository from the project, If I am not wrong, you only use it to share the io and main dispatch to ViewModel therefore I would suggest renaming this class to DispatcherProvider. It's the same as TimeRepository. Do you have any intention to use the current name? Don't hesitate to share your idea.

survey-exam/domain/src/main/java/com/example/nimblesurveys/domain/repository/DispatcherRepository.kt

Lines 5 to 8 in 3ae3ac7

	interface DispatcherRepository {
	fun io(): CoroutineDispatcher
	fun main(): CoroutineDispatcher
	}

• This class is not being invoked from any place, could we get rid of this class?
  

  survey-exam/data/src/main/java/com/example/nimblesurveys/data/api/Retrofit.kt

  Lines 9 to 24 in 3ae3ac7

  	const val BASE_URL = "https://nimble-survey-web-staging.herokuapp.com/"
  	fun getOkHttpClient(): OkHttpClient = OkHttpClient.Builder()
  	.build()
  	fun getRetrofit(client: OkHttpClient): Retrofit = Retrofit.Builder()
  	.baseUrl(BASE_URL)
  	.client(client)
  	.addConverterFactory(MoshiConverterFactory.create())
  	.build()
  	inline fun <reified T> getService(): T {
  	val okHttpClient = getOkHttpClient()
  	val retrofit = getRetrofit(okHttpClient)
  	return retrofit.create(T::class.java)
  	}

• Redundant breakline on SignInRequest class
  

  survey-exam/data/src/main/java/com/example/nimblesurveys/data/api/auth/SignInRequest.kt

  Lines 5 to 16 in 3ae3ac7

  	data class SignInRequest(
  	@Json(name = "grant_type") val grantType: String = "password",
  	@Json(name = "email") val email: String,
  	@Json(name = "password") val password: String,
  	@Json(name = "client_id") val clientId: String,
  	@Json(name = "client_secret") val clientSecret: String
  	)

• The onBackPressed on the MainActivity is not being used.
  

  survey-exam/app/src/main/java/com/example/nimblesurveys/MainActivity.kt

  Lines 46 to 48 in 3ae3ac7

  	override fun onBackPressed() {
  	super.onBackPressed()
  	}

[ryan-conway]

It's good to use a pattern to convert the object to another kind for usage, I see you are using SurveyAdapter and TokenAdapter to convert the object to the entity by mapping the object's properties, so I think it should be mapper class instead of adapter. How do you think?

Calling it an adapter is the convention I'm used to using, but I have no objections to renaming the classes to mapper since that is their main function 😄

It means that we could use the repository class in case working with the data sources. With the DispatcherRepository from the project, If I am not wrong, you only use it to share the io and main dispatch to ViewModel therefore I would suggest renaming this class to DispatcherProvider. It's the same as TimeRepository. Do you have any intention to use the current name? Don't hesitate to share your idea.

I decided on calling them repositories for consistency with AuthRepository and SurveyRepository. I am ok with renaming them to provider, but should they then be moved into another package separate from the repositories?

This class is not being invoked from any place, could we get rid of this class?

Yes we can get rid of this, the only usage is the hardcoded BASE_URL property used by a unit test but this should be removed

Redundant breakline on SignInRequest class
The onBackPressed on the MainActivity is not being used.

These are oversights on my part, will remove both

[hoangnguyen92dn]

Calling it an adapter is the convention I'm used to using, but I have no objections to renaming the classes to mapper since that is their main function 😄

Another approach is we could use Kotlin extension for mapping purposes, i.e: Survey.toEntity(), with this approach we don't need to pass the survey object to adapter/mapper anymore.

should they then be moved into another package separate from the repositories?

👍 We would move to another package

[ryan-conway]

Removed unused code and renamed classes in PR #19

[hoangnguyen92dn]

@ryan-conway Just a minor thing, should we rename the TokenAdapter to Mapper to be consistent with the SurveyMapper? And the package name should not be adapter as well 😄

https://github.com/ryan-conway/survey-exam/blob/9b8b6c131ee557bd94d2bae06e4176d2ea8eae18/data/src/main/java/com/example/nimblesurveys/data/adapter/TokenAdapter.kt