search

kshitijyelpale / blockchain-hyperledger-fabric-electronic-patient-records

GNU General Public License v3.0
49 stars 43 forks source link

CU-cv35qe - Creation of Admin smartcontract and structuring SC #26

Closed varshakamath1 closed 3 years ago

kshitijyelpale commented 3 years ago

Task linked: CU-cv35qe Creation of Admin smartcontract and structuring SC

varshakamath1 commented 3 years ago

Hi,

  • There are still some functions which are duplicate in multiple classes, which restricts us to reuse the single piece of code.
  • I suggested some structuring changes in What's app group chat. Has it some problems with that or any limitations?
  • I am sorry for the comments, but the things can be improved :) If you find this a bit difficult I can do that or help you in that.

Changing into a single piece of code cannot be done because -

  1. Because methods like queryPatientsByLastName, queryPatientsByFirstName, queryAllPatients should only be used by admin and doctor. If you make them abstract, then there should be an implementation in patient contract as well.
  2. The reason for keeping the above functions in both admin and doctor contract, is that the results of these functions are different in both the contracts.
  3. Same explanation goes for getPatientHistory function, it is should be used only by doctor and patient.
  4. It is just the readPatient function that is common to all three contracts.
varshakamath1 commented 3 years ago

image

Hi Varsha,

  • I practiced some OOPS concept here in this smart contract hierarchy. Check it out and tell me how's it and now code a bit reusable ;)
  • I attached patch file (created on your branch) to the ticket, as it is not possible to attach the patch file here.
  • I have some doubts, like in getAllPatientResults() why you stored values in jsonRes.Value when history flag enabled and jsonRes.Record. when false. I tried with storing in Record and it worked. You skipped patient id when history is enabled. Is there any reason? We can revert that if we are breaking with unifying to Record.
  • BTW parent functions can not be called unless the functions definition specified child classes with Super keyword. I observed that oops concepts are not mature in JavaScript unlike other Programming languages

@kshitijyelpale: I am not able to find the patch file in my branch.

Yes, I noticed it too, So, I have already changed jsonRes.Value to jsonRes.Record and added patient Id as well. It will be visible in the next commit.

kshitijyelpale commented 3 years ago

Patch is attched to the ticket.

varshakamath1 commented 3 years ago

image Hi Varsha,

  • I practiced some OOPS concept here in this smart contract hierarchy. Check it out and tell me how's it and now code a bit reusable ;)
  • I attached patch file (created on your branch) to the ticket, as it is not possible to attach the patch file here.
  • I have some doubts, like in getAllPatientResults() why you stored values in jsonRes.Value when history flag enabled and jsonRes.Record. when false. I tried with storing in Record and it worked. You skipped patient id when history is enabled. Is there any reason? We can revert that if we are breaking with unifying to Record.
  • BTW parent functions can not be called unless the functions definition specified child classes with Super keyword. I observed that oops concepts are not mature in JavaScript unlike other Programming languages

@kshitijyelpale: I am not able to find the patch file in my branch.

Yes, I noticed it too, So, I have already changed jsonRes.Value to jsonRes.Record and added patient Id as well. It will be visible in the next commit.

@kshitijyelpale Thank you for the help :)

kshitijyelpale commented 3 years ago

image Hi Varsha,

  • I practiced some OOPS concept here in this smart contract hierarchy. Check it out and tell me how's it and now code a bit reusable ;)
  • I attached patch file (created on your branch) to the ticket, as it is not possible to attach the patch file here.
  • I have some doubts, like in getAllPatientResults() why you stored values in jsonRes.Value when history flag enabled and jsonRes.Record. when false. I tried with storing in Record and it worked. You skipped patient id when history is enabled. Is there any reason? We can revert that if we are breaking with unifying to Record.
  • BTW parent functions can not be called unless the functions definition specified child classes with Super keyword. I observed that oops concepts are not mature in JavaScript unlike other Programming languages

@kshitijyelpale: I am not able to find the patch file in my branch. Yes, I noticed it too, So, I have already changed jsonRes.Value to jsonRes.Record and added patient Id as well. It will be visible in the next commit.

@kshitijyelpale Thank you for the help :)

Welcome