Trjtdtk

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.

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.

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.

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

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)

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