Ismailtaktak / freemedforms

Automatically exported from code.google.com/p/freemedforms
Other
0 stars 0 forks source link

Patienphoto #200

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Like I already said, if the patientmodel does not return a default pixmap, you 
must code in **ALL** patient views the default gender photo (which is a source 
of bug).
If you look at the patientbar widget (viewer of the PatientModel) when patient 
does not have a photo, you don't get the default gender photo...

Original issue reported on code.google.com by berenger...@gmail.com on 21 Oct 2012 at 12:12

GoogleCodeExporter commented 9 years ago
I suggest to add a

    PhotoOrDefaultGenderImage_64x64

in the patient model data representation

Original comment by eric.mae...@gmail.com on 21 Oct 2012 at 12:14

GoogleCodeExporter commented 9 years ago
uh-oh - PhotoOrDefaultGenderImage_64x64 as additional method in the patient 
model - I would highly discourage this. This just blows up the model.

Think of separation of concerns. Think of a software that has a completely 
other GUI and wants to display the patient pictures not als normal pictures, 
but maybe as icons (what you do e.g. in FMF as well in other places).
Then this function of the model would be completely useless.

The model just provides data, and nothing else. if there is no data (no photo), 
provide NULL.
Then the view (AND EACH VIEW!!) has to decide what to do then.

But you are right, not every single view should implement this on its own, this 
is a source of bugs like hell. But it's NOT the responsibility of the MODEL 
layer, it's a VIEW layer responsibility!!

So: As this is needed in many places, I would suggest to implement it in 
ITheme, and add this method, something like 
"ITheme::defaultGenderImage(Gender/int gender, Sizehint sizehint = 
Pixmap64x64)".

The Gender is the big thing that I don't like here. "m", Gender::male, 0?

Then every view can just do 
  BlaBlu->setPimap(theme()->defaultGenderImage(Male));
And everything is ok...
;-)

Original comment by christian.a.reiter@gmail.com on 21 Oct 2012 at 11:00

GoogleCodeExporter commented 9 years ago
or maybe with some more lines:

QPixmap photo = patient()->data(IPatient::Photo_64x64).value<QPixmap>;
BlaBlu->setPimap(photo.isNull() ? theme()->defaultGenderImage(Male) : photo);

Original comment by christian.a.reiter@gmail.com on 21 Oct 2012 at 11:03

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Edit: I implememted this in a separate git branch, please tell me what you 
think of that, here's the patch: http://pastebin.com/1E7ZbC03

Original comment by christian.a.reiter@gmail.com on 22 Oct 2012 at 12:01

GoogleCodeExporter commented 9 years ago
what do you say about that, eric?

Original comment by christian.a.reiter@gmail.com on 28 Oct 2012 at 10:43

GoogleCodeExporter commented 9 years ago
Ah yes I forgot to answer to your  patch. That's a great idea. Just remove the 
theme()-> bacause it link to this object :D

Original comment by eric.mae...@gmail.com on 29 Oct 2012 at 10:06

GoogleCodeExporter commented 9 years ago

Original comment by christian.a.reiter@gmail.com on 29 Oct 2012 at 11:28