Open alessandriniluca opened 3 years ago
MarketingQuestion: minor changes, one doubt: the list of marketingAnswer, maybe can be made EAGER, since basically I think that every time we fetch the question, we need also the answer (but I'm not 100% sure, please confirm)
It's lazy because we do not access the answers using this collection. It's here for convenience, it could be removed, because we do not really use that side of the relation! It is completely useless and we do not even update it. Quando settiamo la risposta la settiamo nel side della answer sempre! E non accediamo mai alle domande dalla parte delle question (che è un po' inutile se ci pensi).
Product: all ok, one question: also here, the fetch type of questionnaire responses, since they are always shown immediately, could have EAGER as fetch type (and since to show them we need to load marketing questions, we may be interested in change into EAGER that fetch type). Another question: why do we need to encode in base64 the image, and wy do we need specificly an indirectlist?`
Image in Base64 it's needed because we cannot save bytes into DB (because a query is text), so to save them we need base64 indirectlist because it's needed in the frontend when he creates adds the MarketingQuestion during the Product Creation page, with that initialization it calls an .addMarketingQuestion(..) when creating it. EAGER because when accessing the inspection we get the list of past products of the day and we do not need the responses, it is bad for performances to load also all the responses with all the past scheduled products
QuestionnaireResponse: minor changes, one question: statisticalAnswer, could be fetch type EAGER
statisticalAnswer answers is not a collection, so no fetch type
User: minor changes, one problem: unique=true in username, I think it is taken into account only during the generation of the DDL. Since we generate this by hand, I think we could remove it.
yeah, it could be removed, but it should be good to put in JavaDoc that it is unique
ProductAdminService: minor changes + Managed the "TODO to release", check if you agree on the brief comment (line 69-73)
I would remove all these lines, because now it's fixed. So, they are not needed.
For what concerns Product and questionnaireResponses it's good to have it LAZY because when we access to the responses we always use a NamedQuery. I don't know if there is something that accesses it using the questionnaireResponses in Product. By the way it's good to keep it, but LAZY
I'll proceed now to remove lines 69-73 of ProductAdminService
. For the rest, I propose to do:
unique=true
, just remember to tell the professor that we left there in order to let other eventual people that manipulates the code know that those attributes are unique;Removed counterpart of unused relationship, leave this issue open for ppt relation
I am checking comments in the backend, please pull. NOTICE: I only changed comments, and unused imports. Nothing in the actual code has been changed all changes made are made to the comments, so the corresponding pull will not affect the behavior of the code
ENTITIES
Log
: all ok;MarketingAnswer
: minor changesMarketingQuestion
: minor changes, one doubt: the list of marketingAnswer, maybe can be made EAGER, since basically I think that every time we fetch the question, we need also the answer (but I'm not 100% sure, please confirm)Offensive
: all ok;Product
: all ok, one question: also here, the fetch type of questionnaire responses, since they are always shown immediately, could have EAGER as fetch type (and since to show them we need to load marketing questions, we may be interested in change into EAGER that fetch type). Another question: why do we need to encode in base64 the image, and wy do we need specificly an indirectlist?QuestionnaireResponse
: minor changes, one question: statisticalAnswer, could be fetch type EAGERStatisticalAnswer
: all ok.User
: minor changes, one problem: unique=true in username, I think it is taken into account only during the generation of the DDL. Since we generate this by hand, I think we could remove it.N.B.: in all entities I removed comments which were left in order to remind us a suggestion of a tag needed in case the mapping boolean-tynyint gave problems. Since we had no problems, I deleted them
UTIL All ok
COMPARATOR All ok, no needs of more comments
EXCEPTIONS All ok, needs of more comments
SERVICES
ProductAdminService
: minor changes + Managed the "TODO to release", check if you agree on the brief comment (line 69-73)ProductService
: minor changesProductUserService
: removed unused importQuestionnaireAdminService
: one comment wrong, + minor changesQuestionnaireResponseService
: removed unused report + minor changesUserService
: minor changes