Store modified class properties

Question

I need to have stored in the class itself a list with (name and value) of the changed properties. However, I don’t know if the form I am using is feasible.

I have the class Employee below:

public class Funcionario
{
    public int IdFuncionario { get; set; }

    public string Nome { get; set; }

    public string Sobrenome { get; set; }

    public string Setor { get; set; }

    public DateTime DataEntrada { get; set; }
}

Create a base class to be able to identify the change and store it:

public abstract class BaseLista
{
    public readonly Dictionary<string, object> Dictionary = new Dictionary<string, object>();

    protected bool SetProperty<T>(ref T storage, T value, [CallerMemberName] String propertyName = null)
    {
        if (Equals(storage, value))
        {
            return false;
        }
        storage = value;
        if (propertyName == null) return true;
        if (!Dictionary.ContainsKey(propertyName))
        {
            Dictionary.Add(propertyName, value);
        }
        else
        {
            Dictionary[propertyName] = value;
        }
        return true;
    }
}

And I changed the staff class this way:

public class Funcionario : BaseLista
{
    private int _idFuncionario;
    private string _nome;
    private string _sobrenome;
    private string _setor;
    private DateTime _dataEntrada;

    public int IdFuncionario
    {
        get { return _idFuncionario; }
        set { SetProperty(ref _idFuncionario, value);}
    }

    public string Nome
    {
        get { return _nome; }
        set { SetProperty(ref _nome, value); }
    }

    public string Sobrenome
    {
        get { return _sobrenome; }
        set { SetProperty(ref _sobrenome, value); }
    }

    public string Setor
    {
        get { return _setor; }
        set { SetProperty(ref _setor, value); }
    }

    public DateTime DataEntrada
    {
        get { return _dataEntrada; }
        set { SetProperty(ref _dataEntrada, value); }
    }
}

Below the test performed:

[TestClass]
public class Testes
{
    [TestMethod]
    public void TesteLista()
    {
        var funcionario = new Funcionario
                              {
                                  Nome = "Paulo",
                                  Sobrenome = "Balbino",
                                  Setor = "Desenvolvimento"
                              };


        var listaPropriedadesAlteradas = funcionario.Dictionary;

    }
}

Is there any better way to do this? I need this list of properties changed to assemble a generic update statement, I don’t want to pass all the entity fields, because I have cases that I won’t have all.

Answer 1

Following the advice mentioned in Maniero’s reply, I used the interface INotifyPropertyChanged in class Funcionario, through the event PropertyChangedEventHandler get the changed properties and store in another object that will compose the message to be sent to WCF Service:

Class Funcionario:

public class Funcionario : INotifyPropertyChanged
{
    public event PropertyChangedEventHandler PropertyChanged;

    private int _idFuncionario;
    private string _nome;
    private string _sobrenome;
    private string _setor;
    private DateTime _dataEntrada;

    public int IdFuncionario
    {
        get { return _idFuncionario; }
        set
        {
            _idFuncionario = value;
            OnPropertyChanged();
        }
    }

    public string Nome
    {
        get { return _nome; }
        set
        {
            _nome = value;
            OnPropertyChanged();
        }
    }

    public string Sobrenome
    {
        get { return _sobrenome; }
        set
        {
            _sobrenome = value;
            OnPropertyChanged();
        }
    }

    public string Setor
    {
        get { return _setor; }
        set
        {
            _setor = value;
            OnPropertyChanged();
        }
    }

    public DateTime DataEntrada
    {
        get { return _dataEntrada; }
        set
        {
            _dataEntrada = value;
            OnPropertyChanged();
        }
    }

    [NotifyPropertyChangedInvocator]
    protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null)
    {
        var handler = PropertyChanged;
        if (handler != null) handler(this, new PropertyChangedEventArgs(propertyName));
    }
}

Test method:

[TestClass]
public class Testes
{
    private Dictionary<string, object> _listaPropriedadesAlteradas = new Dictionary<string, object>();

    [TestMethod]
    public void Teste1()
    {
        var funcionario = new Funcionario();
        funcionario.PropertyChanged += FuncionarioOnPropertyChanged;
        funcionario.Nome = "Paulo Balbino";
        funcionario.Setor = "Desenvolvimento";

        _listaPropriedadesAlteradas.ToList()
                                   .ForEach(c => Debug.WriteLine("Propriedade: {0} - Valor: {1}", c.Key, c.Value));
    }

    private void FuncionarioOnPropertyChanged(object sender, PropertyChangedEventArgs propertyChangedEventArgs)
    {
        _listaPropriedadesAlteradas[propertyChangedEventArgs.PropertyName] =
            sender.GetType()
                  .GetProperty(propertyChangedEventArgs.PropertyName)
                  .GetValue(sender);
    }
}

Now I use the variable _listaPropriedadesAlteradas to compose the message.

Answer 2

Data structure

First I find it odd a class store its own changes. But only you can tell if it really is necessary. I think it hurts at least the principle of single responsibility, and I find it strange a class store things that are not its own, this "history" is not part of an employee. Funcionário is just an employee (better said is an employee’s data). Employee’s change history should be a class that stores this. You can even compose the "history" in the class, although I also think a bad idea.

Another principle that is wounded there is the replacing Liskov. A Funcionário is not a BaseList. It would be better to make an interface, who knows using a utility method of help (Extension method maybe? ). But it would only make sense to use the interface if you still want to hurt the previous principle. I reaffirm that it is right to do this outside this class.

At most I would make a composition rather than inheritance. Then there would be a field that would guard the state but the mechanism to store would be in another class (I don’t know, ClassPropertiesChanges) - which would be used in this field - identical to BaseLista (think).

And there are other wounded principles that could be cited, but I also don’t think that all principles need to be followed always. And fixing one might fix the others. I’m quoting these two because I would probably follow them in this case. But I don’t have all the information from your case. For me it’s the main change you should make.

I still don’t know if it shouldn’t be something completely separate or composition.

Algorithms

SetProperty() returns a boolean that is not used. I don’t know if you need to return something or if you need to use it. You may use it later. Just don’t forget to evaluate this.

Dictionary Besides being a name that is not significant of what it represents in the code (variable names should not say what type they are but what they keep), it seems strange to me that it is public (it could encapsulate in a property) and even stranger it is declared as readonly.

Note that you are writing on her object. Maybe the readonly work in a different way than you expect. Only the variable is read-only, the object contained in it can be written, by any part of the application since it is public. Variable is one thing, its value is another, at least in reference types

I have my doubts if I pass storage as ref It’s a good idea. I’m not going to test in several situations but I’m not sure it would work if I used it in other ways in the future. In the past I would do something like this, today I take more care even if eventually I have to write more code.

I imagine it was purposeful to allow the field to be changed without going through the property that encapsulates it and that done this will not store in that "history". Not wanting to store in some situation can be useful - it can also be misused - but I don’t know if it’s a good idea to change the field directly. In simple things it may cause no problem but if the property starts having other algorithms, ignoring it can be problematic.

You are saving only the last altered value. Is that right? Maybe you are wanting to do something else with it. Maybe you want to implement a undo feature (undo) or something to compare if there has been change in each field. If this is it, I definitely wouldn’t do it this way, I’d make a separate mechanism to control what you need. Deep down I fell for what I said at the beginning.

Finally, you know you don’t need to check if a key is contained in the dictionary to know whether to change it or add it? When you try to assign a value to a key that does not exist the key is created automatically.

Completion

If I spend more time I think I would find other things that can be improved, but again, it might not apply to your specific case, but I hope this already helps.