trjhtr

I am writing spring hibernate app to make online test and this method for calculating user grade I have one to many relations between multi-choice question and answer.

Also, please check this question

@Service
public class StudentServiceImpl implements StudentService 

 private Logger logger = LoggerFactory.getLogger(this.getClass());

 @Autowired
 QuestionRepository questionRepository;

 @Autowired
 UserRepository userRepository;

 @Autowired
 Transformer transformer;

 @Override
 @Transactional
 public void correctAnswer(List<StudentAnswersDTO> questionAnswers) 
 logger.info("****** start correct answers ********");
 Long grade = 0L;
 List<QuestionRightAnswerDTO> rightAnswerDTOS = questionRepository.getRightAnswers();
 for (StudentAnswersDTO questionAnswer : questionAnswers) 
 if (questionAnswer.getType().toLowerCase().equals(AppConstants.MULTI_CHOICE)) 
 if (isCorrectAnswer(questionAnswer.getId(), questionAnswer.getAnswerId(), rightAnswerDTOS))
 grade++;
 
 

 User currentStudent = userRepository.findByMobileNumber(SecurityHelper.getCurrentUser());
 ((Student) currentStudent).setGrade(grade);
 ((Student) currentStudent).setStudentAnswers(transformer.transform(questionAnswers, AnswerSelected.class));

 logger.info("****** end correct answers ********");
 


 private boolean isCorrectAnswer(Long id, Long answerId, final List<QuestionRightAnswerDTO> rightAnswerDTOS) 
 for (QuestionRightAnswerDTO rightAnswer : rightAnswerDTOS) 
 if (id.equals(rightAnswer.getQuestionId()) && answerId.equals(rightAnswer.getAnswerId()))
 return true;
 
 return false;
 

Fist of all I will recommend you to use Constructor based dependency injection. Then I can not see the relation between the Question, QuestionAnswer, and QuestionRightAnswer entities... currently you are selecting all the right answers from the DB which you should NOT do! You have to get all the correct answers for this question only. Use the question.id from QuestionAnswer just to match the QuestionRightAnswer.questionId FK. Also you do take care of the MULTI_CHOICE questions only, but why?

Consider an implementation such as:

@Service
public class StudentServiceImpl implements StudentService 

 private Logger logger = LoggerFactory.getLogger(this.getClass());

 QuestionRepository questionRepository;
 UserRepository userRepository;
 Transformer transformer;

 @Autowired
 public NotificationService(QuestionRepository questionRepository,
 UserRepository userRepository,
 Transformer transformer) 
 this.questionRepository = questionRepository;
 this.userRepository = userRepository;
 this.transformer = transformer;
 

 @Override
 @Transactional
 public void correctAnswer(List<StudentAnswersDTO> questionAnswers) 
 logger.info("****** start correct answers ********");
 Long grade = questionAnswers
 .stream()
 .filter(questionAnswer -> questionAnswer.getType().toLowerCase().equals(AppConstants.MULTI_CHOICE))
 .filter(questionAnswer -> questionRepository
 .getRightAnswersByQuestionId(questionAnswer.getId())
 .stream()
 .anyMatch(rightAnswer -> questionAnswer.getAnswerId().equals(rightAnswer.getAnswerId())))
 .count();

 User currentStudent = userRepository.findByMobileNumber(SecurityHelper.getCurrentUser());
 ((Student) currentStudent).setGrade(grade);
 ((Student) currentStudent).setStudentAnswers(transformer.transform(questionAnswers, AnswerSelected.class));
 

 User currentStudent = userRepository.findByMobileNumber(SecurityHelper.getCurrentUser());
 ((Student) currentStudent).setGrade(grade);

Why not just say

 Student currentStudent 
 = (Student) userRepository.findByMobileNumber(SecurityHelper.getCurrentUser());
 currentStudent.setGrade(grade);

Then you don't have to keep repeating the cast.