Skip to content
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.

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

Quiz selection functionality #57

Merged
merged 27 commits into from
Jun 18, 2022
Merged

Conversation

Nodrex
Copy link
Owner

@Nodrex Nodrex commented May 22, 2022

No description provided.

@Nodrex Nodrex linked an issue May 22, 2022 that may be closed by this pull request
@Nodrex Nodrex changed the title Quiz selection functionality in progress Quiz selection functionality May 22, 2022
@Nodrex Nodrex requested a review from tkhat June 5, 2022 11:33
Comment on lines +77 to +80
fun isCorrectAnswer(
techQuizWrapper: TechQuizWrapper,
possibleAnswerKey: String
) = techQuizWrapper.quiz.correctAnswers[possibleAnswerKey] ?: false
Copy link
Collaborator

Choose a reason for hiding this comment

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

move this method in TechQuiz class

Copy link
Owner Author

Choose a reason for hiding this comment

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

Well TechQuiz is just a model and should stay that way, but is selected answer correct or not is business logic, so it should be in ViewModel, logic of is correct or not can be changed in the future, like if answers will be differentiated with numbers and not letters, just for example, so yea I think fun isCorrectAnswer should remain in ViewModel

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not agree with you. As I know TechQuiz is an object which has question, possible answers and correct answer, so adding one method for checking correctnes of answers, is more modular and convinient. I'm not asking to add it in dto model, but there should be no problem for adding it in presentation model. I hope that logic for determaining if answer is correct or not is same, there this model is used.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I believe you mean domain/model/TechQuiz.kt and not presentation layer. am I right? @kdiakonidze
If we do that then we need an interface with a defined method isCorrectAnswer and I think that is a generalization of TechQuiz.kt which I agree with, but we need more conversation on that, meanwhile I think we can leave it in ViewModel

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes, domain model. If you use isCorrectAnswer method only one place, than ok.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yes for now it is only defined in this viewmodel and is used in only one compose function

techQuizWrapper.multiSelectionWasDone.value = true
}

private fun saveMultiSelectQuizPoint(techQuizWrapper: TechQuizWrapper) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this method also can be moved to TechQuiz

Copy link
Owner Author

Choose a reason for hiding this comment

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

I can say same as in case of isCorrectAnswer function above.
TechQuiz is Model and saveMultiSelectQuizPoint is buisness logic, which can vary in case of different tests

# Conflicts:
#	testomania/src/main/java/com/earth/testomania/technical/presentation/TechnicalTestsScreen.kt
#	testomania/src/main/java/com/earth/testomania/technical/presentation/ui_parts/QuizAnswerUI.kt
#	testomania/src/main/java/com/earth/testomania/technical/presentation/ui_parts/QuizUI.kt
@Nodrex Nodrex requested a review from kdiakonidze June 12, 2022 11:09
@Nodrex Nodrex requested a review from shalva97 June 12, 2022 11:48
@Nodrex Nodrex merged commit a6364ad into main Jun 18, 2022
@Nodrex Nodrex deleted the 56-implement-tech-quiz-functionality branch June 18, 2022 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement tech quiz functionality
3 participants