Sorry clicked something that send that last email...
To finish
if ( obj instance VMInstanceVO )
return (VMInstanceVO)obj
else
return findByIdIncludingRemoved(obj.getId())
Just a thought
Darren
On Thu, Aug 22, 2013 at 1:38 PM, Darren Shepherd <
darren.s.sheph...@gmail.com> wrote:
> These are great points Alex is making here and I was just noticing the
> same thing.
>
> One if the things I've noticed is that most managers inject a lot of
> DAOs. When you look at the code usually about half those DAOs are
> needed only for findById. So if you are consuming an object use the
> interface and use entity manager. Only if you need a specialized
> findBy[SomeSpecialCriteria] should you actually inject the VO's DAO.
>
> The problem with injecting a lot of DAO is the injected members given
> you a quick look at what are the logical dependencies of the
> class/manager. So if you see 15 DAOs you then have to assume that
> that class may modify up to 15 types of entities.
>
> In other projects that have a similar DAO usage style as CloudStack,
> it's been helpful to actually define the DAOs as returning jnterfaces
> only. We may consider such a pattern, or some variation of it. The
> reason I say this, is that if you need to get a VM by instance name
> your going to call the vmDao.findVMByInstanceName() which returns a
> VO. So two problems. 1) The temptation is to use a VO in the code
> and not the interface 2) The class, which is just a consumer now has
> access to a DAO that can write modify.
>
> So one possible pattern could be to seperate the interfaces into
> VMInstanceAccessDAO and VMInstanceDAO where the access interface has
> all find and list methods that return "? extend VMInstance" and the
> other interface has the modify methods. To make this fully work, you
> would probably have to add a getVO() method to the GenericDao that
> will translate an inteface to a VO. So basically
>
> if ( obj instance VMInstanceVO )
>
>
> Darren
>
> On Aug 22, 2013, at 10:58 AM, Alex Huang <alex.hu...@citrix.com> wrote:
>
> > Hi Everyone,
> >
> > I've been doing a lot of refactoring and I noticed the following type of
> code in our code base.
> >
> > Interface VpcService {
> > Vpc getVpc(long id);
> > PrivateGateway getPrviateGateway(long id);
> > }
> >
> > Interface VpcManager extends VpcService {
> > ...
> > }
> >
> > Class VpcManager implements VpcManager {
> > Public Vpc getVpc(long id) {
> > Return _vpcDao.findById(id);
> > }
> > Public PrivateGateway getPrivateGateway(long id) {
> > Return _privateGateway.findById(id);
> > }
> > }
> >
> > CloudStack was specifically written so people don't have to do this.
> It's just useless lines that makes following code harder.
> >
> > I know most schools teach these abstraction concepts and there are valid
> uses but they don't apply in cloudstack. There's certainly a school of
> thought that you need to guard your entities from outside developers. In
> cloudstack, that fence is at the process boundary of cloudstack or, iow,
> that fence is cloudstack's over the wire api. Once code got past that
> layer, code is collaborating and there's no need for that fence. Now, this
> doesn't mean we are advocating all code can modify all entities at will.
> Manager is here to manage the lifecycle and changes to the entities they
> manage. However, it is not there to provide access. Consumers of your
> code should know by convention to use the entity interface instead of the
> VO objects and leave modifications to that manager. So here's some general
> things to think about.
> >
> > If you are writing code for CloudStack's core orchestration work:
> > - Write your managers to manage entities, not provide access
> > - Entity interfaces are for consummation so it shouldn't have set
> methods on them.
> > - Entity interfaces should be in cloud-api which defines CloudStack's
> sdk.
> > - CloudStack's core VO and DAO are in cloud-engine-schema. This forms
> the schema for CloudStack. Note that this is for the core objects
> CloudStack manages and exposes. If you're writing a plugin and the plugin
> has its own DB, there's no need to put that into cloud-engine-schema.
> >
> > If you are writing code for plugins:
> > - If you need to modify certain entities in cloudstack, you can add
> dependency to cloud-engine-schema and have access to the vo and daos. Make
> sure you really need to do this though.
> > - Never assume an interface can be cast down to the VO.
> > - If you are consuming an entity, use the interface not the VO. You can
> use EntityManager to do this. For example, any code can do the following
> after declaring dependency on the cloud-api package.
> >
> > @Inject
> > EntityManager _entityMgr;
> >
> > Vpc vpc = _entityMgr.findById(Vpc.class, vpcId);
> > PrivateGateway pg = _entityMgr.findById(PrivateGateway.class, gatewayId);
> >
> > --Alex
>
