Obvious flaws in my EJB3 design

I have a domain object called VehicleRecord which is a hibernate entity. CRUD operations on these VehicleRecords are handled through an entity access object implemented as a stateless session bean interface

@Local
interface VehicleRecordEao {
    void add(VehicleRecord record);
    List<VehicleRecord> findAll();
    ...
}

@Stateless
class Hibern开发者_开发知识库ateVehicleRecordEaoBean implements VehicleRecordEao { ... }

From a business layer perspective removing and adding these records is more than just a CRUD operation. There may be logging and security requirements for example. To provide these operations to clients sessions beans are created in a business layer.

@Local
interface VehicleRecordManager {
    void createVehicleRecord(VehicleRecord record);
    List<VehicleRecord> findAll(String make, String model); 
    ...
}

@Stateless
class VehicleRecordManagerBean {
    public void createVehicleRecordManager(VehicleRecord record) {
        //business rules such as logging, security,
        //   perhaps a required web service interaction
        //add the new record with the Eao bean
    }
    ...
}

Controllers work between the presentation layer and the above business layer to get work done, converting between presentation objects (like forms) and entities as necessary.

Something here is not right (smells). I have a class called Manager which has got to be a red flag, but several examples in EJB books actually suggest this kind of high level class and tend to use the name manager themselves. I am new to EJB design but in OO design making high level classes called Manager or Handler or Utility screams of procedural and requires one to rethink their design.

Are these procedural-utility-class session beans the normal pattern, or is bad to organize your session by a bunch of methods related only to the entity they operate on? Are there any naming conventions for session beans? Should the Eao's and business session beans work at the same layer?

If there is a less smelly alternative to this pattern I would love to know, thanks.

Your approach is more-or-less standard. Yes, at a fundamental level this is a procedural approach and goes hand-in-hand with what has been dubbed the Anemic Domain Model "anti-pattern". The alternative is to incorporate your business logic into your domain model so as to create a more OO design where your operations are coupled with your DO's. If you were to go down this route you should be aware of the inherent pros and cons. If you feel that your approach makes the most sense, is easy to understand, test, scale, etc... Then role with it. I have worked on several n-tier projects where this exact "procedural" style is used -- rest assured it is quite standard in EE applications to do things this way.

It's an age old discussion. Should a bread bake itself, or does an oven bake it? Does a message sends itself, or does the postoffice do it? Do I send a message to my friend Pete by asking him to send a message to himself?

According to those who came up with the term ADM, it's more "natural" and more "OO" to let each object do all those things itself. Taken to the extreme (which no one advocates of course, but just as an example) class User would contain all logic for the entire application, since ultimately the user does everything.

If you use slim entities (entities containing only data) and service or DAO objects to work with them, then you're not necessarily not doing OO. There is still a lot of OO going around. Services implement interfaces, inherit from base classes, have encapsulated state (e.g. the EntityManager in JPA), have transparent interceptors via dynamic proxies (e.g. to check security and start/commit a transaction), etc etc.

The term "Anemic" has a definite negative association, but calling the same thing "Slim" actually sounds good. This isn't just a case of using different names to hide code smells, but represents a different thinking among different groups of people.

I think it's up to your taste, and depending on the complexity of your application - these things matter less / more. I think in design, you need to simply thing around the line of:

"If someone new came in without any prior knowledge of the system in question - is it intuitive enough for that person to follow and trace the codes, and straight forward to find where things are"

In your case - naming the EJB related to the Entity Object makes it straight forward and simple - what I dont get is why did you separate it to 2 classes EAO and Manager. Why dont you just combine them into one, so if the EJB/Bean class deals with VehicleRecord Entity, then it will be the "VehicleRecordEAO" or "VehicleRecordManager" or "VehicleRecordAccess" or anything really.

I think EAO / DAO / Access sounds more like getter / setter - or any other simple operations. I dont see anything wrong with "Manager" and make it consistent across the board that all business layer will be called "Manager".

Or if you feel better, think of it as the Facade Pattern - so you can call your business layer (the Manager) as VehicleRecordFacade and VehicleRecordFacadeBean.

That way you basically follow the name and concept of Facade pattern, where it becomes intermediary between application layer and the data layer.

Something here is not right (smells). I have a class called Manager which has got to be a red flag

My answer here is towards this concern of yours.

Yes. It is a red flag. Naming a class like "VehicleRecordManager" would be a code smell suggesting that Single responsibility principle would be violated sooner or later.

To elaborate, let me take a few use cases to deal with VehicleRecord

1. Buy a vehicle
2. Rent a vehicle
3. Search for a vehicle
4. Sell a vehicle
5. Find Dealers

In most Java applications when we write up a "VehicleService" ( or "VehicleManager") all the above operations would be placed in this class ! Well, this one is easy to do, but hard to maintain. And certainly this class has a lot of responsibilities, hence many reasons to change. (violating Single responsibility principle )

Would calling it VehicleDao eliminate some of the smelliness? A simple change, but indicates clearly it's concerned with data access concerns.