23

I have a web application. I don't believe the technology is important. The structure is an N-tier application, shown in the image on the left. There are 3 layers.

UI (MVC pattern), Business Logic Layer (BLL) and Data Access Layer (DAL)

The problem I have is my BLL is massive as it has the logic and paths through the application events call.

A typical flow through the application could be:

Event fired in UI, traverse to a method in the BLL, perform logic (possibly in multiple parts of the BLL), eventually to the DAL, back to the BLL (where likely more logic) and then return some value to the UI.

The BLL in this example is very busy and I'm thinking how to split this out. I also have the logic and the objects combined which I don't like.

enter image description here

The version on the right is my effort.

The Logic is still how the application flows between UI and DAL, but there are likely no properties... Only methods (the majority of classes in this layer could possibly be static as they don't store any state). The Poco layer is where classes exist which do have properties (such as a Person class where there would be name, age, height etc). These would have nothing to do with the flow of the application, they only store state.

The flow could be:

Even triggered from UI and passes some data to the UI layer controller (MVC). This translates the raw data and converts it into the poco model. The poco model is then passed into the Logic layer (which was the BLL) and eventually to the command query layer, potentially manipulated on the way. The Command query layer converts the POCO to a database object (which are nearly the same thing, but one is designed for persistence, the other for the front end). The item is stored and a database object is returned to the Command Query layer. It is then converted into a POCO, where it returns to the Logic layer, potentially processed further and then finally, back to the UI

The Shared logic and interfaces is where we may have persistent data, such as MaxNumberOf_X and TotalAllowed_X and all the interfaces.

Both the shared logic/interfaces and DAL are the "base" of the architecture. These know nothing about the outside world.

Everything knows about poco other than the shared logic/interfaces and DAL.

The flow is still very similar to the first example, but it's made each layer more responsible for 1 thing (be it state, flow or anything else)... but am I breaking OOP with this approach?

An example to demo the Logic and Poco could be:

public class LogicClass
{
    private ICommandQueryObject cmdQuery;
    public PocoA Method1(PocoB pocoB) 
    { 
        return cmdQuery.Save(pocoB); 
    }

    /*This has no state objects, only ways to communicate with other 
    layers such as the cmdQuery. Everything else is just function 
    calls to allow flow via the program */
    public PocoA Method2(PocoB pocoB) 
    {         
        pocoB.UpdateState("world"); 
        return Method1(pocoB);
    }

}

public struct PocoX
{
     public string DataA {get;set;}
     public int DataB {get;set;}
     public int DataC {get;set;}

    /*This simply returns something that is part of this class. 
     Everything is self-contained to this class. It doesn't call 
     trying to directly communicate with databases etc*/
     public int GetValue()
     {

         return DataB * DataC; 
     }

     /*This simply sets something that is part of this class. 
     Everything is self-contained to this class. 
     It doesn't call trying to directly communicate with databases etc*/
     public void UpdateState(string input)
     {        
         DataA += input;  
     }
}
  • I don't see anything fundamentally wrong with your architecture as you've currently described it. – Robert Harvey Apr 2 at 14:32
  • 19
    There isn't enough functional detail in your code example to provide any further insight. Foobar examples seldom provide sufficient illustration. – Robert Harvey Apr 2 at 14:40
  • 1
    Submitted to your consideration: Baruco 2012: Deconstructing the framework, by Gary Bernhardt – Theraot Apr 2 at 18:51
  • 4
    Can we find a better title for this question so it can be found online more easily? – Soner Gönül Apr 3 at 10:28
  • 1
    Just to be pedantic : a tier and a layer are not the same thing. A "tier" speaks about deployment, a "layer" about logic. Your data layer will be deployed to both the server-side-code- and database-tiers. Your UI layer will be deployed to both the web-client- and server-side-code-tiers. The architecture you show is a 3-layer architecture. Your tiers are "Web client", "Server side code" and "Database". – Laurent LA RIZZA Apr 4 at 7:03
54

Yes, you are very likely breaking core OOP concepts. However don't feel bad, people do this all the time, it doesn't mean that your architecture is "wrong". I would say it is probably less maintainable than a proper OO design, but this is rather subjective and not your question anyway. (Here is an article of mine criticizing the n-tier architecture in general).

Reasoning: The most basic concept of OOP is that data and logic form a single unit (an object). Although this is a very simplistic and mechanical statement, even so, it is not really followed in your design (if I understand you correctly). You are quite clearly separating most of the data from most of the logic. Having stateless (static-like) methods for example is called "procedures", and are generally antithetic to OOP.

There are of course always exceptions, but this design violates these things as a rule.

Again, I would like to stress "violates OOP" != "wrong", so this is not necessarily a value judgement. It all depends on your architecture constraints, maintainability use-cases, requirements, etc.

  • 9
    Have an upvote, this is a good answer, if I were writing my own, I would copy & paste this, but also add that, if you find your self not writing OOP code, perhaps you should consider a non-OOP language as it comes with a lot of extra overhead that you can do without if you aren't using it – TheCatWhisperer Apr 2 at 17:18
  • 2
    @TheCatWhisperer: Modern enterprise architectures don't throw away OOP entirely, just selectively (e.g. for DTO's). – Robert Harvey Apr 2 at 20:01
  • @RobertHarvey Agreed, I meant if you don't use OOP hardly anywhere in your design – TheCatWhisperer Apr 2 at 21:20
  • @TheCatWhisperer many of the advantages in an oop like c# are not necessarily in the oop part of the language but in the support available such as libraries, visual studio, memory management etc. – Orangesandlemons Apr 7 at 0:09
  • @Orangesandlemons I am sure there are plenty of other well supported languages out there... – TheCatWhisperer Apr 8 at 14:35
31

One of the core principles of Functional Programming is pure functions.

One of the core principles of Object Oriented Programming is putting functions together with the data they act on.

Both of these core principles fall away when your application has to communicate with the outside world. Indeed you can only be true to these ideals in a specially prepared space in your system. Not every line of your code must meet these ideals. But if no line of your code meets these ideals you can't really claim to be using OOP or FP.

So it's OK to have data only "objects" that you fling around because you need them to cross a boundary that you simply can't refactor to move the interested code across. Just know that isn't OOP. That's reality. OOP is when, once inside that boundary you gather all the logic that acts on that data into one place.

Not that you have to do that either. OOP isn't all things to all people. It is what it is. Just don't claim something follows OOP when it doesn't or you're going to confuse people trying to maintain your code.

Your POCO's seem to have business logic in them just fine so I wouldn't worry too much about being anemic. What does concern me is they all seem very mutable. Remember that getters and setters don't provide real encapsulation. If your POCO is headed for that boundry then fine. Just understand this isn't giving you the full benefits of a real encapsulated OOP object. Some call this a Data Transfer Object or DTO.

A trick I've used successfully is to craft OOP objects that eat DTO's. I use the DTO as a parameter object. My constructor reads state from it (read as defensive copy) and tosses it aside. Now I've got a fully encapsulated and immutable version of the DTO. All methods concerned with this data can be moved here provided they're on this side of that boundary.

I don't provide getters or setters. I follow tell, don't ask. You call my methods and they go do what needs doing. They likely don't even tell you what they did. They just do it.

Now eventually something, somewhere is going to run into another boundary and this all falls apart again. That's fine. Spin up another DTO and toss it over the wall.

This is the essence of what the ports and adapters architecture is all about. I've been reading about it from a functional perspective. Maybe it'll interest you too.

  • 5
    "getters and setters don't provide real encapsulation" - yes! – Boris the Spider Apr 3 at 7:34
  • 3
    @BoristheSpider - getters and setters absolutely provide encapsulation, they just don't fit your narrow definition of encapsulation. – Davor Ždralo Apr 3 at 11:58
  • 3
    @DavorŽdralo: They're occasionally useful as a workaround, but by their very nature, getters and setters break encapsulation. Providing a way to get and set some internal variable is the opposite of being responsible for your own state and for acting on it. – cHao Apr 3 at 13:56
  • 5
    @cHao - you don't understand what a getter is. It doesn't mean a method that returns a value of an object property. It's a common implementation, but it can return a value from a database, request it over http, calculate it on the fly, whatever. Like I said, getters and setters break encapsulation only when people use their own narrow (and incorrect) definitions. – Davor Ždralo Apr 3 at 14:50
  • 4
    @cHao - encapsulation means you're hiding implementation. That is what gets encapsulated. If you have "getSurfaceArea()" getter on a Square class, you don't know if surface area is a field, if it's calculated on the fly (return height * width) or some third method, so you can change the internal implementation any time you like, because it's encapsulated. – Davor Ždralo Apr 3 at 21:07
1

If I read your explanation correctly your objects look a bit like this: (tricky without context)

public class LogicClass
{
    private ICommandQueryObject cmdQuery;
    public PocoA Method(PocoB pocoB) { ... }
}

public class PocoX
{
     public string DataA {get;set;}
     public int DataB {get;set;}
     ... etc
}

In that your Poco classes contain only data and your Logic classes contain the methods which act on that data; yes, you have broken the principles of "Classic OOP"

Again, it's hard to tell from your generalised description, but I would hazard that what you have written could be categorized as Anemic Domain Model.

I don't think this is a particularly bad approach, nor, if you consider your Poco's as structs does it nescarly break OOP in the more specific sense. In that your Objects are now the LogicClasses. Indeed if you make your Pocos immutable the design could be considered quite Functional.

However, when you reference Shared Logic, Pocos that are almost but not the same and statics I start to worry about the detail of your design.

  • I've added to my post, essentially copying your example. Sorry ti wasn't clear to start with – MyDaftQuestions Apr 2 at 14:37
  • 1
    what I mean is, if you told us what the application does it would be easier to write examples. Instead of LogicClass you could have PaymentProvider or whatever – Ewan Apr 2 at 15:48
1

One potential problem I saw in your design (and it's very common)--some of the absolutely worst "OO" code I've ever encountered was caused by an architecture that separated "Data" objects from "Code" objects. This is nightmare-level stuff! The problem is that everywhere in your business code when you want to access your data objects you TEND TO just code it right there inline (You don't have to, you could build a utility class or another function to handle it but this is what I've seen happen repeatedly over time).

The access/update code doesn't generally get collected so you end up with duplicate functionality everywhere.

On the other hand, those data objects are useful, for instance as database persistence. I've tried three solutions:

Copying values in and out to "real" objects and throwing away your data object is tedious (but can be a valid solution if you want to go that way).

Adding data wrangling methods to the data objects can work but it can make for a big messy data object that is doing more than one thing. It can also make encapsulation more difficult since many persistence mechanisms want public accessors... I haven't loved it when I've done it but it's a valid solution

The solution that has worked best for me is the concept of a "Wrapper" class that encapsulates the "Data" class and contains all the data wrangling functionality--then I don't expose the data class at all (Not even setters and getters unless they are absolutely needed). This removes the temptation to manipulate the object directly and forces you to add shared functionality to the wrapper instead.

The other advantage is that you can ensure that your data class is always in a valid state. Here's a quick psuedocode example:

// Data Class
Class User {
    String name;
    Date birthday;
}

Class UserHolder {
    final private User myUser // Cannot be null or invalid

    // Quickly wrap an object after getting it from the DB
    public UserHolder(User me)
    {
        if(me == null ||me.name == null || me.age < 0)
            throw Exception
        myUser=me
    }

    // Create a new instance in code
    public UserHolder(String name, Date birthday) {
        User me=new User()
        me.name=name
        me.birthday=birthday        
        this(me)
    }
    // Methods access attributes, they try not to return them directly.
    public boolean canDrink(State state) {
        return myUser.birthday.year < Date.yearsAgo(state.drinkingAge) 
    }
}

Note that you don't have the age check spread throughout your code in different areas and also that you aren't tempted to use it because you can't even figure out what the birthday is (unless you need it for something else, in which case you can add it).

I tend not to just extend the data object because you lose this encapsulation and the guarantee of safety--at that point you might as well just add the methods to the data class.

That way your business logic doesn't have a bunch of data access junk/iterators spread throughout it, it becomes a lot more readable and less redundant. I also recommend getting into the habit of always wrapping collections for the same reason--keeping looping/searching constructs out of your business logic and ensuring they are always in a good state.

1

Never change your code because you think or someone tells you that it’s not this or not that. Change your code if it gives you problems and you figured out a way to avoid these problems without creating others.

So apart from you not liking things, you want to invest a lot of time to make a change. Write down the problems you have right now. Write down how your new design would solve the problems. Figure out the value of the improvement and the cost of making your changes. Then - and this is most important - make sure that you have the time to complete those changes, or you will end up half in this state, half in that state, and that’s the worst situation possible. (I once worked on a project with 13 different types of strings, and three identifiable half-arsed efforts to standardise on one type)

0

The "OOP" category is much larger and more abstract than what you are describing. It does not care about all of this. It cares about clear responsibility, cohesion, coupling. So on the level you are asking, it does not make much sense to ask about "OOPS practice".

That said, to your example:

It seems to me that there is a misunderstanding about what MVC means. You are calling your UI "MVC", separately from your business logic and "backend" control. But for me, MVC includes the whole web application:

  • Model - contains the business data + logic
    • Data layer as implementation detail of the model
  • View - UI code, HTML templates, CSS etc.
    • Includes client-side aspects like JavaScript, or the libraries for "one-page" web applications etc.
  • Control - the server-side glue between all other parts
  • (There are extensions like ViewModel, Batch etc. which I won't go into, here)

There are some exceedingly important base assumptions here:

  • A Model class/objects never has any knowledge at all about any of the other parts (View, Control, ...). It never calls them, it does not assume to be called by them, it gets no sesssion attributes/parameters or anything else along this line. It is completely alone. In languages which support this (e.g., Ruby), you can fire up a manual command line, instantiate Model classes, work with them to your hearts content, and can do everything they do without any instance of Control or View or any other category. It has no knowledge about sessions, users, etc., most importantly.
  • Nothing touches the data layer except through a model.
  • The view has only a light touch on the model (displaying stuff etc.) and nothing else. (Note that a good extension is "ViewModel" which are special classes that do more substantial processing for rendering data in a complicated way, which would not fit well into either Model or View - this is a good candidate for removing/avoiding bloat in the pure Model).
  • Control is as lightweight as possible, but it is responsible for gathering all the other players together, and transfering stuff around between them (i.e., extract user entries from a form and forward it to the model, forward exceptions from the business logic to a useful error messages for the user, etc.). For Web/HTTP/REST APIs etc., all the authorization, security, session management, user management etc. happen here (and only here).

Importantly: UI is a part of MVC. Not the other way round (like in your diagram). If you embrace that, then fat models are actually pretty good - provided that they indeed do not contain stuff they should not.

Note that "fat models" means that all the business logic is in the Model category (package, module, whatever the name in your language of choice is). Individual classes should obviously be OOP-structured in a good way per whatever coding guidelines you give yourself (i.e., some maximum lines-of-code per class or per method, etc.).

Also note that how the data layer is implemented has very important consequences; especially whether the model layer is able to function without a data layer (e.g., for unit testing, or for cheap in-memory DBs on the developer laptop instead of expensive Oracle DBs or whatever you have). But this really is an implementation detail at the level of architecture we are looking at right now. Obviously here you still want to have a separation, i.e. I would not want to see code which has pure domain logic directly interleaved with data access, intensely coupling this together. A topic for another question.

To get back to your question: It seems to me that there is a large overlap between your new architecture and the MVC scheme I have described, so you're not on a completely wrong way, but you seem to be either reinventing some stuff, or using it because your current programming environment / libraries suggest such. Hard to tell for me. So I can't give you an exact answer on whether what you are intending is particularly good or bad. You can find out by checking whether every single "thing" has exactly one class responsible for it; whether everything is highly cohesive and low coupled. That gives you a good indication, and is, in my opinion, enough for a good OOP design (or a good benchmark of the same, if you will).

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.