CODEWITHSUNDEEP
c#oopmetricscohesion
John V
John V

Reputation: 5067

Low cohesion in helper/storage classes, is that really wrong?

Having a class such as this, with two getters for two instance variables:

class A
{
   _fieldA;
   _fieldB;

  GetA()
  GetB()

  GetSpecialNumber(int a)
  {
      //calculation not requiring any fields
  }
}

The class will be classified as lacking cohesion completely. However, I believe in some cases such a stateless object is desired and thus cohesion metric should not apply. Or is that a wrong approach/thinking? Truth is, I have never read about low cohesion being good, except for a few cases mentioned at the end of this material: http://www.aivosto.com/project/help/pm-oo-cohesion.html

Upvotes: 1

Views: 394

Answers (2)

Robert Bräutigam
Robert Bräutigam

Reputation: 7744

No, low cohesion is not good, at least not if you are aiming for object-orientation.

Now, to be fair and to warn you, this is probably not a majority opinion, but DTOs, Beans, Properties, or whatever you call them are not well designed (as the linked article seems to suggest). Again, only if you care about object-orientation. If you don't care that much, then of course you may decide what you want to do.

Obviously there are subtleties, like is a given metric really correct, or whether outside forces (requirements) pull you toward low coupling. However, the reason we want to avoid low coupling is that we want to have things that change together in one place for maintainability. Low coupled things will probably not change together, therefore less maintainability.

Low cohesion sometimes leads to high coupling. For example DTOs (low cohesion objects) lead to high coupling by design. Every time anything in the DTO changes, all the places where that DTO is used need to be examined. That is, those places are highly coupled to the DTO.

Upvotes: 2

Ivan Sanz Carasa
Ivan Sanz Carasa

Reputation: 1387

For this case, I would use properties instead of fields, this helps some tools to understand that this is a DTO (which is a correct thing) so they stop complaining about cohesion and code quality.

struct X
{
    public int A { get; private set; }
    public int B { get; private set; }
}

In case GetSpecialNumber(int a) doesn't use any of the fields/properties, it can be a static method:

public static int GetSpecialNumber(int a)

I would also move it to a helper class if it will be used from somewhere else.

public static class SpecialNumberHelper
{
    public static int GetSpecialNumber(int a)
    {
        // calculation not requiring any fields
    }
}

Upvotes: 1

Related Questions