views:

85

answers:

2

I'm having a design issue I could use some advice on. Let's say we need (how original...) Employees in our new application. What I would normally do is something like this:

public interface IEmployee
{
    string EmployeeId { get; }
    string Name { get; }
    void Update(string newName, ...);
    ...
}

public class Employee : IEmployee
{
    public Employee(string id, string name, ...)
    {

    }
    ...
}

This gets the employees from the data source

public class SqlEmployeeRepository : IEmployeeRepository
{
    ...

    public IEmployee GetEmployee(string id)
    {
        ...
        IEmployee employee = new Employee(id, name, ...);
        return employee
    }

    public IEmployee SaveEmployee(IEmployee employee)
    {
        // Execute SQL command.
    }
}

Visualizing would look something like this:

TextBox nameTextBox = new TextBox();
...
nameTextBox.Text = employee.Name;

And saving would look like this:

string name = nameTextBox.Text;
employee.Update(name, ...);
myEmployeeRepository.Save(employee);

So far, so good. But then I ran this and this article and they made me wonder what the application would look like (statically as well as dynamically) without the getters, so I tried to implement the above application without getter using the technique described in the second article. I came up with this:

public interface IEmployee
{
    public interface Importer
    {
        string ProvideId();
        string ProvideName();
        ...
    }

    public interface Exporter
    {
        void AddId();
        void AddName();
        ...
    }

    void Export(IExporter exporter)
    ...
}

public class Employee : IEmployee
{
    private string _id;

    private string _name;

    public Employee(IEmployee.Importer importer)
    {
        _id = importer.ProvideId();
        _name = importer.ProvideName();
        ...
    }

    public void Export(IEmployee.Exporter exporter)
    {
        exporter.AddId(_id);
        exporter.AddName(_name);
        ...
    }
}

Then the repository becomes:

public class SqlEmployeeExporter : IEmployee.Exporter
{
    ...
    public void Save() { ... }
}

public class SqlEmployeeRepository : IEmployeeRepository
{
    ...

    public IEmployee GetEmployee(string id)
    {
        IEmployee.Importer importer = new SqlEmployeeImporter(id);
        IEmployee employee = new Employee(importer);
        return employee
    }

    public IEmployee SaveEmployee(IEmployee employee)
    {
        SqlEmployeeExporter exporter = new SqlEmployeeExporter();
        employee.Export(exporter);
        exporter.Save();
    }
}

Visualizing becomes:

EmployeeNameTextBoxExporter exporter = new EmployeeNameTextBoxExporter();
employee.Export(exporter);
exporter.Render();

And something similair for saving.

Though the latter implementation removed the necessity for getters on Employee and thus is the better form of data encapsulation it also seems a little bloated and overly complex. What is your opinion on this matter? Am I missing or misinterpreting something in the articles? What is your general opinion on the use of getters (and setters)?

This little experiment makes me tend to using the accessor approach for now. Maybe you can change my mind :-)

+3  A: 

I think that the accessors (getters and setters) is far better, at least in your example. I don't see any advantages using the DTO pattern here and, most important, the code become unreadable. One on the key to write good software is the simplicity: the more the code is simple, the more is readable and maintainable. The patter, in general, should be used when it solves a problem. So you first need to detect a problem and then solve it applying the simplest pattern the solves the problem. The choice uf using DTO pattern should be supported by the fact that it solves a specific problem.

themarcuz
Agreed completely. Properties look much more familiar I think. Readability rules :)
dmitko
+1  A: 

Looking at a short code snippet will naturally appear simplistic and wouldn't call for such encapsulation. As program size grows, I can see this approach saving time during maintenance. I don't have much experience with this approach, so I am unable to give a definite answer, but I think it can provide enough benefit to be worth trying and measuring the results.

For readability, I think there are some advantages to this approach over getters/setters. Assume you want to show the employee's work schedule as a table. Using the "builder" approach, the high-level code remains mostly unchanged:

ScheduleDisplay display = new TableScheduleDisplay();
employee.Export(display);
display.Render();

This is much easier to read than a list of instructions whose main purpose is to pull data from an object and place it into another object. I know at a glance that this code displays the schedule as a table.

Michael Venable