2011-02-05 8 views
3

Когда я нахожу новую идею, я всегда придерживаюсь ее и не вижу никаких слабых сторон. Плохие вещи случаются, когда я начинаю использовать новую идею в большом проекте и позже обнаруживаю некоторых мотылек, что идея была очень плохая, и я не должен использовать ее в каком-либо проекте.Каковы недостатки использования метода, который вызывает делегат для каждой строки в SqlDataReader?

Именно поэтому, имея новую идею и готовность использовать ее в новом крупном проекте, Мне нужно ваше мнение об этом, особенно отрицательное.


В течение долгого времени, мне было скучно набирать снова и снова или скопировать и вставить следующие блоки в проектах, в которых база данных должна быть доступны непосредственно:

string connectionString = Settings.RetrieveConnectionString(Database.MainSqlDatabase); 
using (SqlConnection sqlConnection = new SqlConnection(connectionString)) 
{ 
    sqlConnection.Open(); 

    using (SqlCommand getProductQuantities = new SqlCommand("select ProductId, AvailableQuantity from Shop.Product where ShopId = @shopId", sqlConnection)) 
    { 
     getProductQuantities.Parameters.AddWithValue("@shopId", this.Shop.Id); 
     using (SqlDataReader dataReader = getProductQuantities.ExecuteReader()) 
     { 
      while (dataReader.Read()) 
      { 
       yield return new Tuple<int, int>((int)dataReader["ProductId"], Convert.ToInt32(dataReader["AvailableQuantity"])); 
      } 
     } 
    } 
} 

Так что я сделал небольшой класс, который позволяет писать что-то подобное, чтобы сделать то же самое, что и выше:

IEnumerable<Tuple<int, int>> quantities = DataAccess<Tuple<int, int>>.ReadManyRows(
    "select ProductId, AvailableQuantity from Shop.Product where ShopId = @shopId", 
    new Dictionary<string, object> { { "@shopId", this.Shop.Id } }, 
    new DataAccess<string>.Yield(
     dataReader => 
     { 
      return new Tuple<int, int>(
       (int)dataReader["ProductId"], 
       Convert.ToInt32(dataReader["AvailableQuantity"]); 
     })); 

Второй подход:

  • Короче писать,

  • Легче читать (по крайней мере для меня; некоторые люди могут сказать, что на самом деле это гораздо менее читаемо),

  • Сложнее делать ошибки (например, в первом случае я часто забываю открыть соединение перед его использованием, или я забыл while блок и т. д.),

  • быстрее с помощью Intellisense,

  • Гораздо более уплотнена, особенно для простых запросов.

Пример:

IEnumerable<string> productNames = DataAccess<string>.ReadManyRows(
    "select distinct ProductName from Shop.Product", 
    new DataAccess<string>.Yield(dataReader => { return (string)dataReader["ProductName"]; })); 

После реализации такой вещи простой ExecuteNonQuery, ExecuteScalar и ReadManyRows и в небольшом проекте родового DataAccess<T>.ReadManyRows, я был рад видеть, что код намного короче и проще поддерживать.

я нашел только два недостатка:

  • Некоторые изменения в требованиях потребует тяжелых изменений кода. Например, если есть необходимость добавлять транзакции, это будет очень легко сделать с обычным подходом SqlCommand. Если вместо этого используется мой подход, потребуется переписать весь проект для использования SqlCommand s и транзакций.

  • Незначительные изменения на уровне команд потребуют перехода от моего подхода к стандарту SqlCommand. Например, при запросе только для одной строки необходимо либо расширить класс DataAccess, либо включить код SqlCommand вместо ExecuteReader(CommandBehavior.SingleRow).

  • Может быть небольшая потеря производительности (пока у меня нет точных показателей).

Каковы другие слабые стороны этого подхода, особенно для DataAccess<T>.ReadManyRows?

+2

Для меня сложнее узнать, что происходит во втором блоке кода. – CRice

+4

@CRice - Это можно решить, переработав API. Одной из возможностей было бы использование свободного интерфейса для добавления некоторого синтаксического сахара в код клиента: 'new DataAccess (« выберите отдельное ProductName из Shop.Product »). Где ({" @shopId ", this.Shop.Id}). execute (dataReader => {return (string) dataReader ["ProductName"];}) '. Это не правильно C#, и это смешивает два примера, но я надеюсь, что это демонстрирует тактику. –

+0

@ Давид Харкнесс: спасибо за идею. На самом деле, я нахожу его более читаемым, чем то, что я сделал. –

ответ

2

То, что вы пытаетесь выполнить, приятно, мне действительно нравится такой синтаксис, и я думаю, что он довольно гибкий. Однако я считаю, что вам нужно лучше разработать API.

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

Некоторые предложения синтаксиса (у меня нет компилятора теперь так они в основном идеи):

Используйте анонимные типы вместо словарей

Это тривиально, чтобы написать помощник, который преобразует анонимный тип к но я думаю, что это значительно улучшает обозначение, и вам не нужно будет писать new Dictionary<string, object>.

Использование Tuple.Create

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

Создание сильной типизацией обертку DataReader

Это устранило бы эти уродливые преобразования все вокруг места, и на самом деле, вы действительно должны иметь доступ к DataReader в этом лямбда?

Я проиллюстрирую это кодом для ваших примеров.
Kudos to David Harkness для цепочки идея.

var tuples = new DataAccess ("select ProductId, AvailableQuantity from Shop.Product where ShopId = @shopId") 
    .With (new { shopId = this.Shop.Id }) // map parameters to values 
    .ReadMany (row => 
     Tuple.Create (row.Value<int> ("ProductId"), row.Value<int> ("AvailableQuantity"))); 

var strings = new DataAccess ("select distinct ProductName from Shop.Product") 
    .ReadMany (row => row.Value<string> ("ProductName")); 

Я также могу видеть, что это распространяется на обработку одного выбора строки:

var productName = new DataAccess ("select ProductName from Shop.Product where ProductId = @productId") 
    .With (new { productId = this.SelectedProductId }) // whatever 
    .ReadOne (row => row.Value<string> ("ProductName")); 

Это черновик для класса:

class Row { 
    DataReader reader; 

    public Row (DataReader reader) 
    { 
     this.reader = reader; 
    } 

    public T Value<T> (string column) 
    { 
     return (T) Convert.ChangeType (reader [column], typeof (T)); 
    } 
} 

Он должен быть создан внутри ReadOne и ReadMany звонки и обеспечивает удобный (и ограниченный) доступ к базовому DataReader для selector lambdas.

+0

+1 Мне нравится, где вы это взяли. 'С (new {shopId = this.Shop.Id})' является возвышенным. Одно из предложений состоит в том, чтобы переименовать окончательные методы в 'ReadRows()', 'ReadRow()' и 'ReadValue()' для обработки различных случаев из нескольких строк, одной строки и одного значения. Многие и одно могут вводить в заблуждение, поскольку я вижу Один и думаю «ох, один ряд. Отлично!», Но код выглядит так, как будто он возвращает одно значение. –

1

Мои мысли: вы вставляете SQL в код в строку (в отличие от использования LINQ, который, по крайней мере, проверен синтаксисом, который помогает при сохранении файла сопоставления DBML или EDMX с вашей структурой базы данных) , Внедрение SQL в неэксклюзивный проверенный код таким образом может легко привести к недостижимому коду, в котором вы (или кто-то еще) позже измените структуру базы данных или встроенную строку SQL таким образом, чтобы это нарушило приложение. Встраивание SQL в строки особенно подвержено возникновению труднодоступных ошибок, поскольку код с логическими ошибками будет по-прежнему компилироваться правильно; это делает более вероятным, что разработчики, которые менее знакомы с базой кода, получат ложное чувство безопасности, которое изменило их, не имели каких-либо неблагоприятных или непреднамеренных эффектов.

+0

извините, мой вопрос, вероятно, был недостаточно ясен. Я не сравниваю прямые SQL-запросы с подходом LINQ/EF. Два случая - это либо использование 'SqlCommand' напрямую, либо использование одного и того же' SqlCommand' через мой класс DataAccess.Поэтому в обоих случаях запрос передается как строка. –

1

Ваш подход к абстракции выглядит здорово. Любая производительность, вызванная вызовами дополнительных методов, будет тривиальной, а время разработки намного дороже процессорного времени. Когда вам нужно добавить транзакции или однострочные выборки, вы можете расширить свои классы библиотек. Вы хорошо используете Don't Repeat Yourself здесь.

Spring Framework для Java активно использует эти типы классов шаблонов и помощников, таких как JdbcTemplate и HibernateTemplate, чтобы удалить необходимость написания шаблона кода разработчиками. Идея состоит в том, чтобы написать и проверить его хорошо один раз и повторно использовать его много раз.

1

Прежде всего, никогда не извиняйтесь за не скопированный код.

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

Использование IEnumerable<T> замечательно, так как вы откладываете исполнение до того, когда и если оно потребляется. Однако, пока вы не достигли конца вашего перечисления, соединение остается открытым.

Реализация вашего метода может потреблять всю нумерацию через ToList(), а затем вместо этого возвращает список. Вы даже можете поддержать отложенное выполнение, выполнив небольшой пользовательский счетчик.

Но мне нужно сделать оговорку вокруг этого, убедитесь, что старый код не делает какую-либо магию при перечислении.

0

Делегат делает это немного сложнее прочитать/понять немедленно для кого-то нового для этого подхода, но его должно быть достаточно легко подобрать в конце концов.

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

new DataAccess<string>.Yield(
    dataReader => 
    { 
     return new Tuple<int, int>(
      (int)dataReader["ProductId"], 
      Convert.ToInt32(dataReader["AvailableQuantity"]); 
    })); 

Использование выхода накладывает определенные ограничения на где вы можете попробовать/уловить (Why can't yield return appear inside a try block with a catch?), но это проблема и в предыдущем подходе, и может быть неактуальной в вашем сценарии.