Hi,
am wondering if there is a reason that HibernateProviderDAO uses hibernates load() method which throws an exception if you ask for a row that doesnt exist?
done in:
example:
we use the get() in the other DAO implementations which returns null if row not found.
I would consider this a bug, what do you say?
Calling load seems unconventional, I believe it should be get()
I fully agree!
Getting a persistent by PK is not a finder, it makes sense to throw an ObjectNotFoundException. Is there a use case where the provider id is known but does not exist? Maybe it’s a sign of something not right (an FK missing perhaps).
all our getDomainObject() by ID or UUID use hibernates get() and dont throw exceptions in case of non existence.
@darius do you maybe know why this is the case for provider?
related ticket [TRUNK-5153] - OpenMRS Issues
I can’t think of any reason why I would have intentionally done this 6
years ago. I assume we can switch to get().
Do you prefer a NullPointerException instead of an ObjectNotFoundException? I would say that this case almost never happens (an entity is deleted when it’s open in another user session).
We have ~18 HibernateXXXDAO, of which only one uses hibernates load() method. I would care more about consistency.
You could change all the 18 and be consistent. 
I’d think we shouldn’t be throwing exceptions if an object is not found by a unique identifier, we probably should fix HibernateProviderDAO
just to understand, do you mean:
?
thanks @lluismf 
Yes, that’s what I mean. This kind of search is never performed by the user, it’s internal (WS, form).
Yes, in this case it’s a search performed by a final user. It should return an empty list, not null.
thanks! I do see value in this approach.
and yes @dkayiwa we could of course change all the other DAO implementations, but this would have a bigger impact. ObjectNotFoundException thrown by getOurDomainObject() would need to be caught somewhere and Im sure it isnt at the moment, so I assume UIs would show the exception.
In the past, I used session.load() for to increase performance since session.load() is not hitting database directly and session.get() does. When I use session.load() and if there is no row found, I would handle the ObjecNotFoundException gracefully. I am not sure in this case we need to worry about performance or not. However, performance is always and should be in developer’s mind. Because, we will have to deal with the performance later anyhow.
Yes @dkayiwa, it is noticeable. However, it is acceptable for now. All cloud software as OpenMRS need to address performance. However, it is a very BIG task to tackle for now and it is an on going task. It depends on many components. We can only do our best. Perhaps, we can address it later. Do you know where I can find the link to OpemMRS performance goal page?
Some years back, we had this: https://wiki.openmrs.org/display/RES/Top+Ten+List+for+1.8+Performance+Improvements Then there was this module: https://wiki.openmrs.org/display/docs/System+Performance+and+Utilization+Module We have also had conversations here update performance tests in openmrs-core and here Performance testing of RA
Thank you sir. I will look at all the links as soon as I have a chance.
If the instance is already in session get does not hit the database. It’s the first level cache.
Javadoc:
I agree. If the persistent is not in session, it should be retrieved from database (this will happen just once in the current session). I believe in the 99% of use cases this is fine.
