[
https://issues.apache.org/jira/browse/OPENJPA-317?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12520592
]
Kevin Sutter commented on OPENJPA-317:
--------------------------------------
Patrick,
Thank you for doing this re-factoring. Attempting to make a clean distinction
between API's, SPI's and internal interfaces and classes will be well-received
by all.
Besides the comments that Craig already posted, I have a couple of other
observations.
1. Maybe this was already discussed on another thread and I missed it, but the
naming convention for the SPI interfaces is a bit different from what I am used
to. Instead of appending SPI to the interface name, I'm more used to putting
.spi. in the package name. Something like this:
import org.apache.openjpa.persistence.spi.OpenJPAEntityManager
There are pros and cons to both approaches. The biggest con to this .spi.
approach is when you are using both the API and SPI in the same code block.
One of the references would have to be fully qualified in order to
differentiate. The pro to this .spi. approach is that it looks cleaner when
dealing with the SPI's.
I'm flexible either way. I just thought I would bring this up so that we would
at least consider it.
2. I saw several cases of casting like this:
OpenJPAEntityManagerSPI kem = (OpenJPAEntityManagerSPI) cast(em);
(First of all, should we rename this variables to "oem" or "em" to get rid of
the indirect reference to kodo? :-) )
The real issue is the actual casting. We are calling a method to cast, but
then we're casting the result again? I understand the issue, but it looks kind
of clumsy. Instead, should we introduce a cast() method on the
OpenJPAEntityManagerSPI class? Or, should there be a castSPI() method on the
public API? That doesn't sound quite right, does it? Actually, I have often
wondered what these cast() methods were really used for. In most cases, the
cast() method is doing the same thing as a normal Java cast, so why do we need
these methods? Except in the case of em.getDelegate() calls, but is this
buying us that much?
Just a general issue about whether these cast() methods are really necessary?
And, if they are deemed necessary, then what classes should provide them?
That's it. Thanks!
Kevin
> API formalization pre-1.0
> -------------------------
>
> Key: OPENJPA-317
> URL: https://issues.apache.org/jira/browse/OPENJPA-317
> Project: OpenJPA
> Issue Type: New Feature
> Components: jpa
> Affects Versions: 0.9.0, 0.9.6, 0.9.7
> Reporter: Patrick Linskey
> Fix For: 1.0.0
>
> Attachments: OPENJPA-317.patch
>
>
> This issue tracks the effort to formalize and optimize our API prior to the
> 1.0 release.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
