I'm currently working on updating some old projects (inherited stuff) and I keep running into various small issues with the architecture. Namely monolithic classes too tightly coupled.

One example currently is a giant "results" object that looks something like:

public class Person 
   string Name;
   string Address;
   List<FavoriteThings> favorites;

public class FavoriteThings
  List<ARGBColor> Colors;
  List<Recipe> Food;

Some of these are fairly deep, and some medium to large classes (20-50 objects)

The view uses about 15 fields total.

What principles best describe the code smells I'm picking up? SOLID? Are there real reasons to approach the pattern this way? Or is it an anti-pattern? I want to know if I am over scrutining this code? Or am I on the write path to want to extract / refactor this significantly to more of a MVVM patter with classes with minimal information exposed as needed.

  • The view uses about 15 fields total -- So why not provide only those 15 fields in your ViewModel data-bound class? – Robert Harvey Aug 21 '18 at 22:02
  • @RobertHarvey currently the view model is essentially just bound mapped to an entity framework class. Too tightly coupled and large imo. – AthomSfere Aug 21 '18 at 22:06
  • 2
    Well, that's partly why we have the ViewModel abstraction in MVVM... to get you from entities to data that is usable by a View. – Robert Harvey Aug 21 '18 at 22:22
  • @robertharvey I do agree, but I'm looking for solid language to express the underlying principles of the why if that makes sense – AthomSfere Aug 21 '18 at 22:53
  • 1
    How about Separation of Concerns? If you're using loops and code-behind to populate your UI with data from an entity data model, instead of data binding to observable collections on a View Model, then you don't have the separation of concerns that MVVM provides. Separation of Concerns is mentioned in the second sentence of the Wikipedia article on MVVM. – Robert Harvey Aug 21 '18 at 23:56

It's best to always consider SOLID principles. Because it makes your code simpler and easier to maintain.

As I understand, the Person and FavoriteThing classes are both your domain and data entities and are also exposed to the Presentation. I think that's the first design smell here. We need different objects to represent the same concept in different layers. Those classes are designed with tables and data structure in mind. So there may be some problems between your data and domain layers already. I mean maybe those 15 (more or less) fields are enough for the domain, but other fields live there because of table structures or data concerns!

So the first issue we have here is the SOC. These classes are domain classes and handle domain logic but ViewModels are presentation classes and handle presentation logic. So there are different concerns here. Separate them! And also it happens many times that ViewModels need more or less data that exists in multiple domain objects. It's really hard to fill ViewModels with different kinds of domain objects. You can use ServiceLayer and DTOs for this problem. One more thing, do not change your domain entities because of ViewModels or any other presentation object. Domain objects change only because of business needs.

It kind of violate the OCP too I think. Because when your classes have more and more clients they will have more and more reasons for change. And that makes it hard to consider OCP.

So first of all respect the principles and separate codes that have different concerns. Then start refactoring wherever you think that needs it! That makes you consider domain concerns when you're refactoring domain objects, and presentation concerns when you're refactoring ViewModels or other presentation objects.

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.