Resolve "as a admin,i want to read match table so that i can know all data and find out the errors that may occurred"
1.crud for back-end system 2.adjust before code
Merge request reports
Activity
changed milestone to %Sprint 2
added In Progress label
requested review from @c24007548
mentioned in commit 5d54c979
changed milestone to %Sprint 3
removed In Progress label
requested review from @c24058946
- The
@RestController
annotation is appropriate for APIs returning JSON data. - Mixing
@RestController
and methods returningModelAndView
for views (Thymeleaf in this case) can lead to confusion. Consider separating these into distinct controllers:- Use a
@RestController
for API endpoints. - Use a
@Controller
or@RestController
for view-related methods.
- Use a
- The
67 // MatchItemForm emptyMatchItem = new MatchItemForm(); 68 // modelAndView.addObject("matchItem", emptyMatchItem); 69 // return modelAndView; 44 70 // } 45 //matchItemForm add match 46 @GetMapping("/match/add") 47 public ModelAndView addMatchItem() { 48 ModelAndView modelAndView = new ModelAndView("match/matchItemForm"); 49 MatchItemForm emptyMatchItem = new MatchItemForm(); 50 modelAndView.addObject("matchItem", emptyMatchItem); 51 return modelAndView; 52 } 53 71 54 72 // 55 @GetMapping("/match/edit/{id}") 56 public ModelAndView editMatchItemTh(@PathVariable("id") Long id) { 32 matchService.addMatchItem(matchItem); 33 } 34 35 // update 36 @PutMapping("/{id}") 37 public void updateMatchItem(@PathVariable Long id, @RequestBody MatchItem matchItem) { 38 matchItem.setId(id); 39 matchService.updateMatchItem(matchItem); 40 } 41 42 //delete 43 @DeleteMapping("/{id}") 44 public void deleteMatchItem(@PathVariable Long id) { 45 matchService.deleteMatchItem(id); 46 } 47 Your code adheres to RESTful API design principles, using POST/PUT/DELETE to map different operations with clear path naming, parameter names, and straightforward logic. The code is concise, but there are some areas for improvement:
-
Input Parameter Validation:
The validation forMatchItem
is lacking basic checks, such as verifying whether required fields are present or if field values are valid. You could use the@Valid
annotation in combination with@RequestBody
to implement parameter validation effectively. This would help ensure data integrity and reduce potential issues during runtime. -
Return Values:
Currently, all methods returnvoid
, which makes it difficult for the frontend to determine whether an operation was successful. It is recommended to return a standardized response structure, such as an object containing a status code, message, and data. This approach not only improves clarity but also aligns with best practices for API design, facilitating error handling and debugging.
-
Input Parameter Validation:
39 throw new IllegalArgumentException("Invalid Match Item ID"); 40 } 41 MatchItem existingItem = matchRepository.getMatchItemById(matchItem.getId()); 42 if (existingItem != null) { 43 matchRepository.updateMatchItem(matchItem); 44 } else { 45 throw new IllegalArgumentException("Match item not found"); 46 } 47 48 } 49 // delete 50 @Override 51 public void deleteMatchItem(Long id) { 52 matchRepository.deleteMatchItem(id); 53 } 35 54 } Your code demonstrates defensive programming by performing null and invalid value checks on
matchItem.getId()
within theupdateMatchItem
method. Additionally, it shows good encapsulation by delegating database operations to the Repository layer, maintaining a clear separation of concerns. However, there are areas for improvement:-
Custom Exceptions for Business Logic Errors:
UsingIllegalArgumentException
for business logic errors, such as"Match item not found"
, is not semantically appropriate. It’s better to define a custom exception class (e.g.,MatchItemNotFoundException
) that conveys more specific intent about the nature of the error. This improves readability and makes the codebase easier to maintain. -
Logging on Exceptions:
Currently, there is no explicit logging when exceptions occur. Adding logging statements at the point where exceptions are thrown can significantly improve the ability to diagnose and resolve issues. Including relevant context in the log (e.g., thematchItem
ID) can further enhance traceability.
-
Custom Exceptions for Business Logic Errors:
82 matchItem.getStatus(), 83 matchItem.getScoreA(), 84 matchItem.getScoreB(), 85 matchItem.getConfirmByA(), 86 matchItem.getConfirmByB(), 87 matchItem.getWinnerId(), 88 matchItem.getId() 89 ); 90 } 91 92 @Override 93 public void deleteMatchItem(Long id) { 94 String sql = "DELETE FROM match_item WHERE id = ?"; 95 jdbc.update(sql, id); 96 } 73 97 } Your SQL implementation is concise and clear, with logically correct statements and well-mapped relationships between fields and tables. Additionally, the use of
jdbc.update
improves performance by avoiding unnecessary object wrapping overhead. However, there are a few areas for improvement:-
Handling
null
Values in SQL Mappings:
The current implementation lacks checks fornull
values in field mappings, which may lead to SQL exceptions or unexpected behavior. Ensure that nullable fields are handled properly, either by setting default values or explicitly checking fornull
before executing the SQL statement. -
Logging for SQL Operations:
Adding logging for SQL statements and their parameters before and after execution would help in debugging and diagnosing issues. Include the SQL query and the parameter values in the log for better traceability. -
Consistency in Naming Conventions:
The naming convention for SQL fields and object properties (e.g.,player_AId
vsplayerAId
) is inconsistent. This can reduce code readability and increase the likelihood of errors. Standardizing on a consistent naming style, such as camelCase for object fields and snake_case for SQL columns, is recommended to improve maintainability.
-
Handling
Your CSS code adheres to principles of style separation and responsive design. However, there are areas where it can be further improved:
-
Reducing Redundancy:
Styles common tobackgroundCSS
andmatchDetailCSS
(e.g.,*
andbody
) are duplicated. These shared styles can be extracted into a global stylesheet to avoid repetition and ensure consistency across components. -
Unnecessary Nesting:
In.title-h1
, the nested selectors likeh1
andp
are unnecessary. Instead, directly use.title-h1
for theh1
styles and.title-h1 p
for thep
styles. This simplifies the CSS and avoids overly specific selectors. -
Using CSS Variables:
Replace hardcoded color values (e.g.,dodgerblue
) with CSS variables (e.g.,--primary-color
). This makes it easier to update the theme and ensures consistent usage of colors across the codebase. -
Responsive Enhancements:
If responsiveness is achieved using media queries, consider centralizing these queries or using a consistent breakpoint system (e.g.,--breakpoint-sm
,--breakpoint-md
).
-
Reducing Redundancy:
Your JavaScript code is functional but could benefit from improvements in defensive programming and reducing redundancy. Here are some suggestions:
-
Input Validation:
BothaddMatch
andupdateMatch
methods lack validation for form inputs. Before submitting, validate that all required fields are filled and that their values are within acceptable ranges. -
Extract Common Logic:
The form-handling logic inaddMatch
andupdateMatch
is repetitive. Extracting it into a reusable method improves maintainability and reduces duplication. -
Better Error Handling:
Ensure that any errors during submission, such as network failures or API errors, are handled gracefully. Provide meaningful error messages to the user for better debugging and experience. -
Consider Asynchronous Validation:
For fields that depend on server-side checks (e.g., uniqueness constraints), consider integrating asynchronous validation before form submission.
-
Input Validation: