2
\$\begingroup\$

Any advice on how to make this code; cleaner, more effective, just overall better!

Program setup object that is coming from the database EF. It then maps the object to a view model and converts datetime to string and different datetime components.

//EF Data
            List<MeetingEvent> Meeting = new List<MeetingEvent>();
            Meeting.Add(new MeetingEvent() { MeetingId = 1, MeetingName= "A1",StartDateTime= new DateTime(2020, 1, 01),EndDateTime = new DateTime(2020, 1, 05) });
            Meeting.Add(new MeetingEvent() { MeetingId = 2, MeetingName = "A2", StartDateTime = new DateTime(2020, 2, 01), EndDateTime = new DateTime(2020, 2, 05) });
            Meeting.Add(new MeetingEvent() { MeetingId = 3, MeetingName = "A3", StartDateTime = new DateTime(2020, 3, 01), EndDateTime = new DateTime(2020, 3, 05) });
            Meeting.Add(new MeetingEvent() { MeetingId = 4, MeetingName = "A4", StartDateTime = new DateTime(2020, 4, 01), EndDateTime = new DateTime(2020, 4, 05) });
            Meeting.Add(new MeetingEvent() { MeetingId = 5, MeetingName = "A5", StartDateTime = new DateTime(2020, 5, 01), EndDateTime = new DateTime(2020, 5, 05) });
//Logic
            var listOfEvents = Meeting.Select(x => new MeetingEventViewModel
            {
                MeetingId = x.MeetingId,
                MeetingName = x.MeetingName,
                StartDateTime = x.StartDateTime,
                EndDateTime = x.EndDateTime,
                StartDateDayName = x.StartDateTime.DayOfWeek.ToString(),
                StartDateMonth = x.StartDateTime.Month.ToString(),
                StartDateDay = x.StartDateTime.Day.ToString(),
                StartDateYear = x.StartDateTime.Year.ToString(),
                EndDateDayName = x.EndDateTime.DayOfWeek.ToString(),
                EndDateMonth = x.EndDateTime.Month.ToString(),
                EndDateDay = x.EndDateTime.Day.ToString(),
                EndDateYear = x.EndDateTime.Year.ToString(),
            }).ToList().OrderBy(o => o.StartDateTime);

            foreach (MeetingEventViewModel m in listOfEvents)
            {
                Console.WriteLine(m.MeetingId);
            }

    class MeetingEvent
    {
        public int MeetingId { get; set; }
        public string MeetingName { get; set; }
        public DateTime StartDateTime{ get; set; }
        public DateTime EndDateTime { get; set; }
    }

    class MeetingEventViewModel
    {
        public int MeetingId { get; set; }
        public string MeetingName { get; set; }
        public DateTime StartDateTime { get; set; }
        public DateTime EndDateTime { get; set; }
        public string StartDateDayName { get; set; }
        public string StartDateDay { get; set; }
        public string StartDateYear { get; set; }
        public string StartDateMonth { get; set; }
        public string EndDateDayName { get; set; }
        public string EndDateDay { get; set; }
        public string EndDateYear { get; set; }
        public string EndDateMonth { get; set; }
 

}

\$\endgroup\$
2
\$\begingroup\$

First of all I would advice to use DateTimeOffset instead of DateTime.

Use the time, date, datetime2 and datetimeoffset data types for new work. These types align with the SQL Standard. They are more portable. time, datetime2 and datetimeoffset provide more seconds precision. datetimeoffset provides time zone support for globally deployed applications. Source: https://docs.microsoft.com/en-us/sql/t-sql/data-types/datetime-transact-sql?view=sql-server-ver15

Besides that; If you have to do a lot of mapping I would use AutoMapper.

The mapping profiles (where you can do some mapping configuration) would look like this:

  public class MeetingEventProfile : Profile
  {
    public MeetingEventProfile()
    {
        CreateMap<MeetingEvent, MeetingEventViewModel>()
            .ForMember(x => x.StartDateDayName, x => x.MapFrom(_ => _.StartDateTime.DayOfWeek))
            .ForMember(x => x.StartDateMonth, x => x.MapFrom(_ => _.StartDateTime.Month))
            .ForMember(x => x.StartDateDay, x => x.MapFrom(_ => _.StartDateTime.Day))
            .ForMember(x => x.StartDateYear, x => x.MapFrom(_ => _.StartDateTime.Year))
            .ForMember(x => x.EndDateDayName, x => x.MapFrom(_ => _.EndDateTime.DayOfWeek))
            .ForMember(x => x.EndDateMonth, x => x.MapFrom(_ => _.EndDateTime.Month))
            .ForMember(x => x.EndDateDay, x => x.MapFrom(_ => _.EndDateTime.Day))
            .ForMember(x => x.EndDateYear, x => x.MapFrom(_ => _.EndDateTime.Year));
    }
  }

You only see some specific mappings for properties that don't match by name (or require some conversion/etc). Property names that match you don't have to explicitly map; although there is a lot of configuration possible here!

In your logic (which is not poluted with mapping anymore) you would do something like this:

public class MyClass{
  private readonly IMapper _mapper;

  public MyClass(IMapper mapper){
    _mapper = mapper;
  }

  public void MyMethod(){
    var meetingVms = _mapper.Map<MeetingEventViewModel[]>(Meeting);
  }
}

The performance impact by using AutoMapper is very low; I did quite some benchmarks with high performance requirements and the impact was very low.

My example assumes you are using dependency injection. AutoMapper is very easy to implement. Otherwise you have to create the Mapper instance yourself which isn't hard too. The docs are well written: https://docs.automapper.org/en/stable/Dependency-injection.html

\$\endgroup\$

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.