0

I have a class that only holds data, specifically Collections of data, it is called *BezierSplineData:

class BezierSplineData
{
    public List<BezierSplineControlPoint> ControlPoints; // ControlPoints that can change the shape of the spline.
    public List<BezierSplinePoint> Points; // Actual points in the 3D world that represent the spline.
}

BezierSplineControlPoint has only 3 fields:

  • Position
  • FirstTangentPosition
  • SecondTangentPosition

BezierSplinePoint only 1:

  • Position

Another class called BezierSplineCalculator, it is used to calculate 3D points for a set of BezierControlPoint's.

class BezierSplineCalculator
{
    public BezierSplineData Data;

    private void Recalculate()
    {
        // Code that will recalculate the Data.Points based on the Data.ControlPoints;
        // Essentially generates the actual spline.
    }
}

Now the BezierSplineCalculator class can have methods such as, AddControlPoint(BezierSplineControlPoint controlPoint), and other methods like RemoveControlPoint, InsertControlPoint and probably more, this would cause a recalculation of the spline.

My question is, it doesn't look right that I have a separate class just for that as I would need to refer to that Data quite often, let's say I need to render that BezierSpline, I would create a BezierSplineRenderer class and access the Data field from the BezierSplineCalculator class.

Is this a code smell taking in consideration that these classes are related(closely i guess), should I keep the data and calculator in one class?

5

Sometimes.

Occasionally, this separation is weird and limiting and creates a bunch of overhead when you always use the two together. It is not traditional “Car has Wheels” OO.

But these days with more functional programming ideals coming into OO, it can be good. Data Transfer Objects (DTOs) and Plain Ol’ C/Java Objects are these sort of simple structures of data that are then consumed by other things. This can offer a lot of flexibility, and provides a lot of benefit when the data needs to be serialized to other machines or to disk (like data so often does).

For this scenario, I would expect the renderer to have some heafty dependencies on drawing libraries. And I could see scenarios where you have utilities that don’t actually draw the things, just manipulate them. Separating the data from the drawing can let you break that dependency.

Or you’re never going to do that and it’s all overkill.

In general, I’d err towards more, smaller classes.

  • What do you think of having one class called BezierSplineComputer, that would contain the necessary data as well as the functionality like adding control points, the renderer would then depend on the computer class to get the needed data to display the spline on the screen – Joao Vitor Dec 22 '18 at 20:14
1

What you are describing is known as an "Anemic Domain Model" as described by Martin Fowler.

https://www.martinfowler.com/bliki/AnemicDomainModel.html

Typically, this design pattern falls under the not-preffered category.

  • Ten years ago, folk might have taken that article seriously. But times change and ideas move on. These days, Fowler’s our of date ideas around data-only objects “falls under the not preferred category” – David Arno Dec 22 '18 at 22:57
  • @DavidArno see Casablancas answer which, for this case, refutes the "data objects are o.k." theory. And supports Fowler's "out of date" ideas. – user949300 Dec 23 '18 at 6:35
1

Something useful to think about here is the class invariant: what are the constraints on the possible states of an object?

In this case, Points is derived from ControlPoints, so these two fields are not independent. By making the fields public, it is possible for an external client to set them to values that don't actually make sense in the context of a Bezier spline. This is one of the primary reasons for encapsulation, it allows you to expose carefully chosen methods that are guaranteed to maintain the class invariant.

Therefore, I'd say it makes sense to have a single class called BezierSpline, that has methods to add/remove control points, and get the actual points (but no direct way to set the actual points).

  • 2
    So if our team agree to handle data only objects as immutable, then we have no problems ;). – Fabio Dec 23 '18 at 6:53
  • @Fabio Blanket statements like that aren't useful in software design. My answer pointed out a specific design principle that is useful in the context of the OP's question. If you made everything immutable, you'd likely end up violating a bunch of other principles, but I can't say which ones without more context. – casablanca Dec 24 '18 at 2:24
0

What you are describing is a collection. So I would look at other collections.

When you look at for instance: Arraylist, methods for mutating the collection are part of the class. Think about add, insert, sort.

Methods for mutating the collection-elements are not part of the class.

To not surprise your successors I would stay with C#-way and add the class-mutators.

  • It’s not a collection, it’s a spline. They are totally different things. A spline is a mathematical object with a very specific meaning. – gnasher729 Dec 24 '18 at 13:02
0

Whenever you have classes with names like SomethingData, SomethingHandler, SomethingComputer, SomethingCalculator, or the more general pattern of Something+Verb, you have a code smell.

Generally, you'd be better served by cutting down on the Verb classes and use a Noun class: BezierSpline, and move the verbs to become public methods of the Noun class: BezierSpline.getPoints(), BezierSpline.render(), etc. This follows the principle of "tell, don't ask".

  • Yeah i agree, i moved on to calling it just a bezierspline and having that data already inside it, but i guess the issue with your solution is too many responsibilities, because at the end a bezierspline should only be a series of points, no rendering or other stuff i guess. – Joao Vitor Dec 24 '18 at 16:15

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.