Skip to content

database entities definition#101

Open
lonlajordan wants to merge 33 commits intomainfrom
poll_management
Open

database entities definition#101
lonlajordan wants to merge 33 commits intomainfrom
poll_management

Conversation

@lonlajordan
Copy link
Copy Markdown
Contributor

No description provided.

@lonlajordan lonlajordan requested a review from Pattykev April 30, 2023 02:40
public class PollController {
//private final Po
@GetMapping("{id}")
public String display(@PathVariable long id, Model model){
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

God method.
This method has too much responsabilities.

question4.setPropositions(Arrays.asList(p1, p2, p3, p4));
poll.getQuestions().add(question4);
model.addAttribute("poll", poll);
return "poll";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No condition before returning "poll" ? Why this constant and why returning this value ?

}

public static String format(Question question){
StringBuilder builder = new StringBuilder("<h5>").append(StringUtils.defaultString(String.valueOf(question.getNumber()), "#"));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bad naming for builder.


public static String format(Question question){
StringBuilder builder = new StringBuilder("<h5>").append(StringUtils.defaultString(String.valueOf(question.getNumber()), "#"));
builder.append(") ").append(question.getFormulation()).append("</h5>");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Best to display the append iteration with multilines.

builder.append(") ").append(question.getFormulation()).append("</h5>");
switch (question.getAnswerType()){
case DATE:
builder.append("<input type='date' name='question").append(question.getId()).append("'>");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe you should prevent the case where parameters values could change or being injected, there's even the possibility to internationalize these values.

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.

2 participants