3
\$\begingroup\$

I am now developing a product, which will use Unity & Repositories when it comes to data management. I will demonstrate on a simple example. What we want to do, is to create an app with calendar control, where user (e.g. manager) will be able to store holidays for his employees.

First define our models:

We have a class Employee which looks like following:

public class Employee :ViewModelBase, IEditableObject
{
    public int EmployeeID { get; set; }

    public string FullName
    {
        get
        {
            return $"{this.Name} {this.Surname}";
        }
    }

    private string _Name;
    public string Name
    {
        get { return _Name; }
        set
        {
            _Name = value;
            OnPropertyChanged("Name");
        }
    }

    private string _Surname;
    public string Surname
    {
        get { return _Surname; }
        set
        {
            _Surname = value;
            OnPropertyChanged("Surname");
        }
    }

    private DateTime _DateOfBirth;
    public DateTime DateOfBirth
    {
        get
        {
            if (_DateOfBirth == DateTime.MinValue)
                _DateOfBirth = DateTime.Now;
            return _DateOfBirth;
        }
        set
        {
            _DateOfBirth = value;
            OnPropertyChanged("DateOfBirth");
        }
    }
    //hierachy level
    private int _EmployeeLevelID;
    public int EmployeeLevelID
    {
        get { return _EmployeeLevelID; }
        set
        {
            _EmployeeLevelID = value;
            OnPropertyChanged("EmployeeLevelID");
        }
    }

    private ObservableCollection<Holidays> _Holidays;
    public ObservableCollection<Holidays> Holidays
    {
        get
        {
            if (_Holidays == null)
                _Holidays = new ObservableCollection<Shared.Holidays>();
            return _Holidays;
        }
        set
        {
            _Holidays = value;
            OnPropertyChanged("Holidays");
        }
    }
}

public class Holidays : ViewModelBase, ICloneable
{
    public DateTime Start { get; set; }
    public DateTime End { get; set; }
    public string Reason { get; set; }//Holiday, Sick Leave, ...
    public string Description { get; set; }
    public string UniqueID { get; set; }

    public object Clone()
    {
        var item = new Holidays();
        item.Start = this.Start;
        item.End = this.End;
        item.Reason = this.Reason;
        item.Description = this.Description;

        return item;
    }
}

Every Employee has a list of his own "Holidays" (days, when he is not working either because of holidays, or sickness, or any other reason).

Then we have another model for Appointment, which is used under our calendar control. It's quite complex, but here is just a glimpse of it:

  public abstract class AppointmentBase
 {
     protected AppointmentBase();
     public virtual ICategory Category { get; set; }
     public virtual DateTime End { get; set; }
     public virtual Importance Importance { get; set; }
     public virtual bool IsAllDayEvent { get; set; }
     public virtual IRecurrenceRule RecurrenceRule { get; set; }
     public virtual ResourceCollection Resources { get; }
     public virtual DateTime Start { get; set; }
     public virtual string Subject { get; set; }
     public virtual ITimeMarker TimeMarker { get; set; }
     public virtual TimeZoneInfo TimeZone { get; set; }
     public event EventHandler RecurrenceRuleChanged;
     public override void CopyFrom(IAppointment other);
     public bool Equals(IAppointment other);
     protected virtual void OnRecurrenceRuleChanged(EventArgs args);
 }

Here is how our repository could look like:

public class EmployeeRepository : Interfaces.Repository.IEmployeeRepository
{
    #region Repositories
    private readonly ObservableCollection<Employee> repository_Employees;
    #endregion

    private readonly IDataManager _EmployeeManager;
    public EmployeeRepository(IDataManager EmployeeManager)
    {
        _EmployeeManager = EmployeeManager;

        repository_Employees = _EmployeeManager.LoadEmployees();
    }

    public ObservableCollection<Employee> ReturnEmployees()
    {
        return new ObservableCollection<Employee>(repository_Employees);
    }

    public void SaveEmployee(Employee person)
    {
        if (repository_Employees.Any(n => n.EmployeeID == person.EmployeeID))
        {
            //person exists
            var existingPerson = repository_Employees.FirstOrDefault(n => n.EmployeeID == person.EmployeeID);
            existingPerson = person;
        }
        else
        {
            //person does not exist
            repository_Employees.Add(person);
        }

        SaveEmployees();
    }

    public void RemoveEmployee(Employee person)
    {
        //check if ID has already been added to the database
        if (repository_Employees.Any(n => n.EmployeeID == person.EmployeeID))
            repository_Employees.Remove(repository_Employees.First(n => n.EmployeeID == person.EmployeeID));

        SaveEmployees();
    }

    public void SaveEmployees()
    {
        _EmployeeManager.SaveEmployees(repository_Employees);

    }

    public int GetNextEmployeeID()
    {
        if (repository_Employees.Count == 0)
            return 1;
        else
            return repository_Employees.Max(n => n.EmployeeID) + 1;
    }

}

And finally we have a model which loads & saves our data from Repository to Database. Here is how interface would look like:

public interface IDataManager
{
    ObservableCollection<Employee> LoadEmployees();
    void SaveEmployees(ObservableCollection<Employee> employees);
    void DeleteEmployee(Employee person);

    ObservableCollection<EmployeeLevel> LoadEmployeeLevel();
    void SaveEmployeeLevels(ObservableCollection<EmployeeLevel> levels);
    void DeleteEmployeeLevel(EmployeeLevel level);
}

Here comes the tricky part I am not quite sure about. Remember, that we are using a Calendar control, which expects Appointment class. Due to it's complexity (Calendar control is taken from Telerik) I have decided to create a small class Holidays that is present under each employee only with the necessary information (I don't need most of those things that are already implemented under that class).

Due to that, when I load & close my ViewModel, what I need to do is to convert my values to a corresponding model before I can actually save them.

I would like to know the following:

  1. Is this a good way how to implement repository in my application?
  2. Is it a good practice to do conversion the way I have presented?

I am open to any suggestions & comments as I am still learning repository patterns.

\$\endgroup\$
  • \$\begingroup\$ Are you using an object/relational mapper? Entity Framework? \$\endgroup\$ – Mathieu Guindon Mar 8 '16 at 15:24
3
\$\begingroup\$

Your Employee class is a ViewModel, concerned with your application's business-level logic: it's much more than a DTO/POCO class, and using it as such sounds very much like mixing up data and business concerns.

That said...

OnPropertyChanged("Surname");

I'm surprised of seeing literal strings in C# 6.0 code, when nameof is available to make your code refactoring-friendly and strongly-typed:

OnPropertyChanged(nameof(Surname));

Your private fields are not following the naming conventions; they should be _camelCase, not _PascalCase. I like the underscore prefix though, because it eliminates the need to use this as a qualifier.


I'm also a bit surprised to see IClonable implemented in C# 6.0 code, since that interface has been controversial for just about as long as it has existed.

From MSDN:

Because callers of Clone cannot depend on the method performing a predictable cloning operation, we recommend that ICloneable not be implemented in public APIs.

Besides, why clutter up client code with an object when you could just implement a copy constructor?

public Holiday(Holiday existing)
{
    Start = existing.Start;
    End = existing.End;
    Reason = existing.Reason;
    Description = existing.Description;
}

Notice I renamed the type Holiday - you should keep pluralized type names for IEnumerable things (collections, lists, dictionaries, etc.). If I write code against your API, I expect Holidays to represent a bunch of Holiday objects.


Back to higher-level architecture. I think what you're calling your repository is actually just a service that's sitting between your actual data access and your business logic - and IDataManager (really? out of all names?) is your actual repository. And its responsibilities are mixed, too; you can tell just by looking at the interface - here, I've rearranged the members to illustrate:

public interface IDataManager
{
    ObservableCollection<Employee> LoadEmployees();
    ObservableCollection<EmployeeLevel> LoadEmployeeLevel();

    void SaveEmployees(ObservableCollection<Employee> employees);
    void SaveEmployeeLevels(ObservableCollection<EmployeeLevel> levels);

    void DeleteEmployee(Employee person);     
    void DeleteEmployeeLevel(EmployeeLevel level);
}

An interface shouldn't be designed to change. What happens when (note: not if) you need a new entity type? You need to change the interface and modify all implementations (of which, I suspect there's only one, right?). It's literally screaming:

"My cohesion! My cohesion is suffering! My cooooheeeesiooooon!!!"

A more cohesive interface would look like this:

public interface IDataManager<T>
{
    IEnumerable<T> Load();
    void Save(IEnumerable<T> entities);
    void Delete(T entity);     
}

Notice the coupling just went down as well: the interface is no longer tied to any specific concrete types.

High cohesion, low coupling: that's what you should be striving for.

I can't help but notice the similarities:

public interface IRepository<T>
{
    IEnumerable<T> GetAll();
    void Insert(T entity);
    void Update(T entity);
    void Delete(T entity);     
}

That's why I said earlier:

[...] and IDataManager (really? out of all names?) is your actual repository.

– Mat's Mug 5 minutes ago


I would suggest you get your ViewModel classes out of the data layer, and introduce a set of entity classes for your data layer to work with: then your service layer can "translate" entities into view models (and vice-versa) for the presentation layer to play with.

\$\endgroup\$
  • \$\begingroup\$ This is one of the most comprehensive answers I have ever received. Thank you very much. After reading your comments I now see what I can improve. I learned this by myself and therefore never thought about it this way \$\endgroup\$ – Robert J. Mar 8 '16 at 18:45

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.