44 lines
1.1 KiB
Markdown
44 lines
1.1 KiB
Markdown
|
---
|
||
|
|
type: practical
|
||
|
|
---
|
||
|
|
## Genre
|
||
|
|
- Cool, nice. We will extend.
|
||
|
|
|
||
|
|
## FileService
|
||
|
|
- `importBooks` doesn't actually save books
|
||
|
|
- CSV parsing might fail if there are commas in the data itself
|
||
|
|
- Hardcoded CSV column names (just an observation, not necessarily bad)
|
||
|
|
- File creation creates temp files without deleting them
|
||
|
|
|
||
|
|
## BookService
|
||
|
|
- `createBook` doesn't ensure isbn uniqueness
|
||
|
|
|
||
|
|
## BookRepository
|
||
|
|
- Our documentation is not updated. The class map cointains findByISBN.
|
||
|
|
- Only 'isbn' is supported. Implement:
|
||
|
|
- findByAttributes(BookAttributes attrs)
|
||
|
|
- findByID(long pk)
|
||
|
|
- deleteByAttributes
|
||
|
|
- deleteByID
|
||
|
|
|
||
|
|
## BookAttributes
|
||
|
|
- Every field should be private (@Data automatically adds gsetters)
|
||
|
|
- Perhaps add data validation? Not necessary.
|
||
|
|
|
||
|
|
|
||
|
|
## Book
|
||
|
|
- Attributes can be null. Add null checks in getters and setters to avoid. We don't want null ptr exceptions
|
||
|
|
|
||
|
|
## BookController
|
||
|
|
- Do not interact with repository directly. Go through service.
|
||
|
|
|
||
|
|
|
||
|
|
## BookService
|
||
|
|
- `createBook` returns true always, no matter what the status is
|
||
|
|
- matchingAny is OR, false positives might be returned. Perhaps use matchingAll? Not sure about this.
|
||
|
|
- `deleteBooksByAttribute` is missing
|
||
|
|
|
||
|
|
|
||
|
|
|
||
|
|
|