openjpa-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Kevin Sutter (JIRA)" <j...@apache.org>
Subject [jira] Commented: (OPENJPA-317) API formalization pre-1.0
Date Fri, 17 Aug 2007 15:44:30 GMT

    [ 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.


Mime
View raw message