Это хороший стиль C#?

Question

Рассмотрим метод следующую подпись:

public static bool TryGetPolls(out List<Poll> polls, out string errorMessage) 

Этот метод выполняет следующие действия:

• доступ к базе данных, чтобы сформировать список объектов опроса.
• возвращает true, если это было успешным, и errorMessage будет пустой строкой
• возвращает false, если оно не было успешным, а errorMessage будет содержать сообщение об исключении.

Это хороший стиль?

Update: Допустим, я действительно использую метод следующую подпись:

public static List<Poll> GetPolls() 

и в этом методе, он не улавливает никаких исключений (так я зависит вызывающему перехватывать исключения). Как я удаляю и закрываю все объекты, находящиеся в области этого метода? Как только генерируется исключение, код, который закрывает и удаляет объекты в методе, больше недоступен.

Answer 1

Этот метод пытается сделать три вещи:

1. Загружать и возвращающие список опросов
2. возвращают логическое значение, указывающее на успех
3. Вернуться сообщение об ошибке

Это довольно грязный с точки зрения дизайна.

Лучше было бы объявить просто:

public static List<Poll> GetPolls() 

Тогда пусть этот метод бросить Exception если что-то пойдет не так.

Answer 2

Я считаю

public static bool TryGetPolls(out List<Poll> polls) 

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

Answer 3

Как правило, я бы сказал, нет.

Причина, по которой я говорю «нет», на самом деле не потому, что вы выполняете TryGetX и возвращаете bool с параметром out. Я думаю, что это плохой стиль, потому что вы также возвращаете строку с ошибкой.

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

Вместо того, что вы ищете есть пара способов:

public static bool TryGetPolls(out List<Poll> polls); 
public static List<Poll> GetPolls(); 

Таким образом, пользователь может делать то, что уместно и GetPolls может быть реализован в терминах TryGetPolls. Я предполагаю, что ваш static имеет смысл в контексте.

Answer 4

Рассмотрим возвращение:

• пустая коллекция
• нуль

Несколько из параметров, для меня, это code smell. Метод должен делать только ОДИН ВИД.

рассмотреть вопрос о повышении и обработки сообщений об ошибках с:

throw new Exception("Something bad happened"); 
//OR 
throw new SomethingBadHappenedException(); 

Answer 5

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

Однако, возможно, вы просто хотите вернуть «мета» информацию о попытке, и в этом случае вам просто нужен способ вернуть более одной части информации из одного вызова метода. В этом случае я предлагаю создать класс PollResponse, который содержит два свойства: Список < Опрос> Опросы и строка ErrorMessage. После чего ваш метод возвращает объект PollResponse:

class PollResponse 
{ 
    public List<Poll> Polls { get; } 
    public string MetaInformation { get; } 
} 

Answer 6

Зависит от того, если ошибка обычное событие или, если это нам действительно исключение.

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

Если ошибка является частью общей нормальной работы, которая является реальной частью обычной операции, как ошибка, охватывающая строку целым числом в методе int.TryParse, метод, который вы создали, будет более уместным.

Я предполагаю, что первый, вероятно, лучший случай для вас.

Answer 7

Это, безусловно, не идиоматический способ написания C#, что также означает, что он, вероятно, тоже не является хорошим стилем.

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

Если у вас просто есть метод GetPolls, значит, вы всегда хотите получить результаты, и если это не удастся, вы хотите знать, почему в форме Exception.

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

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

IList<Poll> GetPolls(); 

или

bool TryGetPolls(out IList<Poll> polls); 

(Обратите внимание, что я возвращаюсь к IList<Poll>, а не List<Poll> в любом случае тоже, так как это тоже хорошо практика программирования для абстракции, а не для реализации.)

Answer 8

Нет, с моей точки зрения, это очень плохой стиль. Я бы написал это следующим образом:

public static List<Poll> GetPolls(); 

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

Answer 9

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

Для работы с базами данных я думаю, что Exception является лучшим.

Answer 10

Я бы повторил это так.

public static List<Poll> GetPolls() 
{ 
    ... 
} 

Это, вероятно, следует бросать исключение (ErrorMessage), если это не удается получить опросы, плюс это позволяет метод построения цепочки, которая является менее громоздким, чем иметь дело с параметрами из.

Если вы запустите FxCop, вы захотите изменить List на IList, чтобы он был счастлив.

Answer 11

Не совсем - я вижу ряд проблем с этим.

Прежде всего, метод звучит так, как будто вы обычно ожидаете его успеха; ошибки (не могут подключаться к базе данных, не могут получить доступ к таблице polls и т. д.) были бы редкими. В этом случае гораздо разумнее использовать исключения для сообщения об ошибках. Образец Try... предназначен для случаев, когда вы часто ожидаете, что вызов «сбой» - например. при анализе строки на целое число имеет хорошие шансы, что строка является введенной пользователем, что может быть недействительным, поэтому вам нужно иметь быстрый способ справиться с этим - следовательно, TryParse. Здесь не так.

Во-вторых, вы сообщаете об ошибках как значение bool, указывающее наличие или отсутствие ошибки, и строковое сообщение. Как бы вызывающий мог различать различные ошибки? Он, конечно, не может совпадать с текстом сообщения об ошибке - это деталь реализации, которая может быть изменена и может быть локализована. И может быть мир различий между чем-то вроде «Невозможно подключиться к базе данных» (возможно, просто откройте диалоговое окно настроек подключения базы данных в этом случае и разрешите пользователю его редактировать?) И «Подключен к базе данных, но он говорит« Доступ запрещен », ». Ваш API не дает отличного способа отличить их.

Подводя итог: используйте исключения, а не bool + out string для сообщения сообщений. Как только вы это сделаете, вы можете просто использовать List<Poll> в качестве возвращаемого значения, без необходимости аргумента out. И, конечно, переименуем метод на GetPolls, так как Try... зарезервирован для шаблона bool+out.

Answer 12

Я думаю, что это нормально. Я предпочел бы, хотя:

enum FailureReasons {} 
public static IEnumerable<Poll> TryGetPolls(out FailureReasons reason) 

Так строки ошибок не живут в коде доступа к данным ... Методы

Answer 13

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

например. в общественном статическом классе:

public static List<Poll> Fill(this List<Poll> polls) { 
    // code to retrieve polls 
} 

Затем, чтобы назвать это, вы могли бы сделать что-то вроде:

List<Poll> polls = new List<Poll>().Fill(); 
if(polls != null) 
{ 
    // no errors occur 
} 

редактирования: я только что сделал это. вам может понадобиться новый оператор в: List<Poll>().Fill()

Answer 14

Просьба указать ваши предположения, ограничения, желания/цели и аргументы; мы должны угадать и/или читать ваши мысли, чтобы узнать, каковы ваши намерения.

при условии, что вы хотите, чтобы ваша функция

• создать объект списка опросы
• Погасить все исключения
• указывают на успех с логическим
• и обеспечивают дополнительное сообщение об ошибке при сбое

, то указанная выше подпись в порядке (хотя проглатывание всех возможных исключений не является хорошим p ractice).

Как общий стиль кодирования, он имеет некоторые потенциальные проблемы, как упомянули другие.

Answer 15

Рекомендации сказать, чтобы попытаться избежать реф и Out параметры, если они абсолютно не требуется, потому что они делают API труднее использовать (не более цепочки методов, разработчик не должен объявлять все переменные перед вызова метода)

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

Лучший способ объявить метод аналогичен этому.

public static List<Poll> GetPolls() ... 

и ошибок использования отчетов обработки исключений

try 
{ 
    var pols = GetPols(); 
    ... 
} catch (DbException ex) { 
    ... // handle exception providing info to the user or logging it. 
} 

Answer 16

Существует также такая картина, как видно во многих функциях Win32.

public static bool GetPolls(out List<Poll> polls) 


if(!PollStuff.GetPolls(out myPolls)) 
    string errorMessage = PollStuff.GetLastError(); 

Но ИМО это ужасно. Я бы пошел на что-то исключение, если этот метод не должен запускаться 65 раз в секунду в физическом движке 3d-игры или когда-либо.

Answer 17

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

public static IList<Poll> GetPolls() 
{ 
    try 
    { 
    } 
    finally 
    { 
     // check that the connection happened before exception was thrown 
     // dispose if necessary 
     // the exception will still be presented to the caller 
     // and the program has been set back into a stable state 
    } 
} 

На дизайн стороне записки, я бы рассмотреть толкая этот метод в хранилище класса, так у вас есть какой-то контекст, с которым, чтобы понять метод. Предположительно, все приложение не несет ответственности за хранение и получение опросов: это должно быть ответственностью хранилища данных.