3

I've already asked "Dealing with a large interface".

I have a further question regarding that situation. It was pointed out to me that I used a lot of getters and setters and so I broke encapsulation. I'm aware of this, but I can't really imagine how I would code this differently. All my getters seem natural to me - they expose information that I directly react on in the UI. How could it be improved in this way?

The code from the original question:

public interface ISystemSolver
{
    // Configuration
    int NumberBase { get; set; }
    bool CaseSensitive { get; set; }
    bool LeadingZeros { get; set; }

    // State
    IReadOnlyList<int> ForbiddenValues { get; }
    IReadOnlyList<char> ForbiddenVariables { get; }
    event EventHandler OnTooManyVariables;
    bool AreThereTooManyVariables { get; }

    // Variables
    IEnumerable<Variable> Variables { get; }
    void RemoveVariable(int index);
    void AddVariable(char variable, int value);

    // Equations
    IEnumerable<Equation> Equations { get; }
    void RemoveEquation(int index);
    void AddEquation(string equation);

    // Solving
    IEnumerable<Solution> Solve();
}

EDIT: Usage of ForbiddenValues and ForbiddenVariables

private void UpdateVariablesComboBox()
{
    // Get the forbidden variables
    var forbiddenVariables = _manager.ForbiddenVariables;

    // Get the variables that are not forbidden
    var items = _manager.Variables
        .Where(c => !forbiddenVariables.Contains(c))
        .Select(c => c.ToString())
        .Cast<object>()
        .ToArray();

    // Clear the combo box
    _variablesComboBox.Items.Clear();

    // Update the combo box
    _variablesComboBox.Items.AddRange(items);
}

private void UpdateValuesComboBox()
{
    // Get the current number base
    var currentBase = _manager.NumberBase;

    // Get the forbidden values
    var forbiddenValues = _manager.ForbiddenValues;

    // Get possible values that are not forbidden
    var items = Enumerable.Range(0, currentBase)
        .Where(i => !forbiddenValues.Contains(i))
        .Select(i => i.ToString())
        .Cast<object>()
        .ToArray();

    // Clear the combo box
    _valuesComboBox.Items.Clear();

    // Update the combo box
    _valuesComboBox.Items.AddRange(items);
}
13

Getters

Getters turn an object inside out. They say give this data to everyone. I'm just a place to store info. If you want something done about this stuff you have to do it.

Encapsulation

Rather then give up your state to anything that asks it's better to hide it. Anything that needs to be done with that state is better done by methods of the class that holds that state. This gathers data and relevant methods together. Then rather then ask how hot it is and have to decide to heat or cool the house you can just say thermostat.on().

Boundries

This is hard to see how to do for most people because we so often want to expose state across some boundary like displaying it in a GUI, saving it, or sending it to some port.

Indeed to send data out to these things it must be exposed. But that doesn't mean your data should live exposed. I'd rather see thermostat.display() then thermostat.getTemp().

So how could you make that work? Well if your view provided an interface that allowed things to call it then rather then exposing their state to just anyone that asks they could very deliberately send the state just to the view. The view could perform whatever needed logic (C vs. F) before sending it to the UI (which really shouldn't have any logic).

Done this way it's easy to limit how exposed your state is. We only consented to letting the view have it because we were crossing boundary. The other side of that boundary has behavior needs that can't be pulled into the thermostat object. In this case the behavior is the user seeing the temperature. Hard to suck that inside. But things like turning the heat or AC on can be sucked in and so have no reason to be running around being implemented elsewhere.

Polymorphism

The really powerful thing about working this way is polymorphism. thermostat.on() could have all sorts of implementations depending on what thermostat is set to. Sure it could just be a different temperature but it could also be a completely different implementation of on().

Write your code this way and you find you can make all sorts of changes amazingly easily. Which after all is the point of software: easy changes. That's what keeps software soft.

Removing getters

So look hard at everything that uses a getter. What is it doing with it once it gets it? Is it making a decision based on what it got? If so that's behavior to consider sticking in the object. If not is it going out across some boundary? If so let the boundary offer methods that accept what they want sent to them. That way they don't have to ask and they don't have to know what to ask.

Those two activities should seriously reduce the need for getters. You'll start creating behavior objects that really do hide their state. But sometimes you really just need a bag of data that you can fling around. That's ok. We call those value objects. String is the most popular one. Just be sure that your value object is just that. Values. Don't make weird hybrids that are trying to be both. Use value objects for things that cross boundaries together.

Different names for the same thing

This is not a new idea. In fact this is the very heart of object oriented programming. It's been called polymorphism many places, "Tell, don't ask" by Andy Hunt, Dave Thomas & Martin Fowler, and old school OOPers like Alec Sharp call it "message passing". When you don't do it Martin Fowler even has a smell named for it called The Feature Envy Smell.

Details

When you use a getter what you're getting is a detail. You don't want details. You want something done by something that knows how to do it. Spare me the details.

5
  • OP provided a sample code. It would be nice if you provided a modified version of such code using the technique you propose. "Show me the code". I'm not the downvoter, by the way. – Tulains Córdova Aug 5 '17 at 7:33
  • 4
    @TulainsCórdova I could do that but I'm going to need some more help from the OP precisely because I can't tell what uses these getters are put to. Show me what uses the ForbiddenValues and i'll show you some methods that could encapsulate that behavior. – candied_orange Aug 5 '17 at 8:47
  • You can come up with a trivial example of your own with no implementation details, just the bare idea. – Tulains Córdova Aug 5 '17 at 19:27
  • @CandiedOrange I have edited the post. – Patrik Bak Aug 7 '17 at 10:50
  • The problem with this answer is it is entirely generic. It is not specific to the question. – Frank Hileman Aug 10 '17 at 22:45
1
  1. Do all of these mutable properties need to be mutable? I suspect not, having worked on solvers myself.
  2. Take the "configuration" mutable properties, and turn them into immutable fields in your class. Initialize them using a constructor. This leads to...
  3. Stop using an interface if you can use a base class instead. This will enable your to use constructors to initialize immutable fields.
  4. What does your solver do? Does it need to exist for a period of time longer than that needed to solve the equation? I suspect not. You should then consider the solver to be a type of operation object: you initialize it, set it loose, and monitor its progress. You do not modify its state ever. This is a more robust way to approach operations, and allows you to use more immutability.
  5. When the solver completes, it can return a result object that contains information about the attempt to do a solution.
  6. You have no documentation so I cannot tell what the variable properties mean exactly. But it may be that the process of determining variable correctness can be delegated to another class.

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.