Как улучшить Entity Framework и Javascript Interaction

Question

Это довольно расплывчатый / субъективный вопрос.Я хочу знать, является ли это лучшим способом для отправки / получения данных в / из браузера с помощью вызовов ajax.На серверном веб-сервисе я хочу использовать структуру сущностей.Ниже приведены два примера функций.

Критерием «наилучшего» является скорость написания кода, читаемый код и надежная архитектура.

Спасибо за любые отзывы, предложения и комментарии.

Функция Get

[WebMethod]
public AjaxEmployee EmployeeGetById(int employeeID, bool getTimeOff)
{
    using (Time_TrackerEntities ctx = new Time_TrackerEntities())
    {
        var results = from item in ctx.Employees
                      where item.ID == employeeID
                      orderby item.Last_Name
                      select new AjaxEmployee
                      {
                          ID = item.ID,
                          Employee_ID = item.Employee_ID,
                          First_Name = item.First_Name,
                          Middle_Name = item.Middle_Name,
                          Last_Name = item.Last_Name,
                          Supervisor_ID = item.Supervisor_ID,
                          Active = item.Active,
                          Is_Supervisor = item.Is_Supervisor
                      };
        var emp = results.FirstOrDefault();
        if (getTimeOff)
        {
            var results2 = from item2 in ctx.Time_Off
                           where item2.Employee_ID == emp.Employee_ID
                           select new AjaxTime_Off
                           {
                               ID = item2.ID,
                               Employee_ID = item2.Employee_ID,
                               Date_Off = item2.Date_Off,
                               Hours = item2.Hours
                           };
            emp.Time_Off = results2.ToList<AjaxTime_Off>();
        }

        return emp;
    }
}

Функция сохранения

[WebMethod]
public bool EmployeeSave(AjaxEmployee emp)
{
    using (Time_TrackerEntities ctx = new Time_TrackerEntities())
    {
        var results = from item in ctx.Employees
                      where item.ID == emp.ID
                      select item;

        var myEmp = results.FirstOrDefault();
        if (myEmp == null)
        {
            myEmp = new Employee();
            ctx.Employees.AddObject(myEmp);
        }

        myEmp.Employee_ID = emp.Employee_ID;
        myEmp.First_Name = emp.First_Name;
        myEmp.Middle_Name = emp.Middle_Name;
        myEmp.Last_Name = emp.Last_Name;
        myEmp.Supervisor_ID = emp.Supervisor_ID;
        myEmp.Active = emp.Active;
        myEmp.Is_Supervisor = emp.Is_Supervisor;

        return ctx.SaveChanges() > 0;
    }
}

Answer 1

Необходимо сделать несколько улучшений.

Метод Save () - Не копируйте слева направо, используйте встроенную логику EF

Вместо этого:

myEmp.Employee_ID = emp.Employee_ID;
myEmp.First_Name = emp.First_Name;
myEmp.Middle_Name = emp.Middle_Name;
myEmp.Last_Name = emp.Last_Name;
myEmp.Supervisor_ID = emp.Supervisor_ID;
myEmp.Active = emp.Active;
myEmp.Is_Supervisor = emp.Is_Supervisor;

Вы можете сделать это:

ctx.Employees.ApplyCurrentValues(emp).

Что это делает, так это ищет сущность с тем же ключом в графе (который существует, поскольку вы только что получили его с помощью FirstOrDefault()), и переопределяете скалярные значения передаваемой вами сущностью - именно то, что вы делаете.

Таким образом, ваши 7 строк становятся 1, плюс, если вы добавите какие-либо дополнительные скалярные свойства - вам не придется рефакторинг вашего кода. Только помните - работает только для скалярных свойств , а не навигационных свойств .

Зачем строить запрос для поиска первичного ключа? Просто используйте предикат для SingleOrDefault ()

Вместо этого:

var results = from item in ctx.Employees
              where item.ID == emp.ID
              select item;

var myEmp = results.FirstOrDefault();

Сделайте это:

var myEmp = ctx.Employees.SingleOrDefault(x => x.ID == emp.Id);

Или, что еще лучше, используйте трубу / фильтр Техника:

var myEmp = ctx.Employees.WithId(emp.Id).SingleOrDefault();

Где WithId - метод расширения IQueryable<Employee>, который фильтрует запрос на основе предоставленного идентификатора сотрудника. Это позволяет отсоединить фильтрацию / бизнес-логику от вашего хранилища / DAL. Он должен соответствовать вашей модели домена, поэтому вы можете иметь хороший свободный API для запроса сущностей вашего домена через ORM.

Когда вы извлекаете сущность с помощью первичного ключа, вы должны всегда использовать SingleOrDefault() или Single(), никогда FirstOrDefault() или First(). Если это первичный ключ - должен быть только один из них, поэтому вы должны выбросить исключение, если существует более одного, что и делает SingleOrDefault(). И, как упоминает @Shiraz, ваш FirstOrDefault() приведет к сбою запроса ниже. При использовании <First/Single>OrDefault().

вам всегда нужна проверка на ноль.

Те же улучшения могут быть сделаны в вашем методе Get.

В целом, нет ничего функционально неправильного в вашем коде - он просто нуждается в незначительных улучшениях, нулевой проверке и обработке исключений.

Единственное функциональное улучшение, которое я настоятельно рекомендую , - это преобразование кода вашего веб-сервиса в общий репозиторий. Поскольку код очень тривиален и может быть повторно использован для любого объекта. Веб-сервис не должен касаться транзакций, первичного ключа или логики EF. Там даже не должно быть ссылки на EF DLL. Инкапсулируйте эту логику в хранилище и делегируйте ей логику постоянства (конечно, через интерфейс).

После внесения изменений, о которых я упоминал выше, методы веб-службы должны содержать не более 5-7 строк кода.

У вас слишком много интеллекта в вашем веб-сервисе - он должен быть тупым и упорным невежественным.

Answer 2

Я нахожу, что обычно довольно плохая идея пытаться использовать мои объекты непосредственно в контракте на данные. Это возможно, и в некоторых случаях работает нормально, но каждый раз, когда моя объектная модель становится даже немного сложной, я начинаю беспокоиться о графе объектов так, как мне этого не хочется.

Вместо этого, и это независимо от клиента, но оно также применимо к клиенту JS, я пытаюсь представить классы контрактов данных как чистые средства передачи данных (DTO) без отображения в EF вообще. Эти классы - просто документы, которые я передаю взамен, тело сообщения, если хотите. Они могут переводиться в команды для моей модели, или они могут использоваться для заполнения запроса, или что-то еще, но они не являются самими сущностями.

Это, на мой взгляд, сильно упрощает вещи. Когда вы впервые пишете простой сервис, вам может показаться, что кода больше, но с течением времени это значительно облегчает обслуживание.

И в качестве дополнительного примечания, вам также следует подумать о том, чтобы лучше разделить свои обязанности. Класс веб-службы не должен нести ответственность за непосредственное создание и удаление контекста данных, он должен зависеть от DAO или интерфейса репозитория (или службы домена), который обрабатывает все это для вас (и применяет транзакции по мере необходимости и т. Д.).

Answer 3

Ваш метод get может аварийно завершить работу.

Если эта строка возвращает ноль:

   var emp = results.FirstOrDefault();

Тогда эта строка завершится с ошибкой нулевой ссылки:

   where item2.Employee_ID == emp.Employee_ID

Я также добавил бы некоторые блоки try catch с ведением журнала ошибок.