C#→ Критика класса LiveInternet
Янв 12, 2011

После первого опыта написания класса и программы на C#, я попросил знатоков этого языка оценить свежеиспеченный код. Для этого был направлен вопрос на форум сайта RSDN. Спустя несколько минут, я получил полный и развернутый ответ с указанием на все недочеты и узкие места в моем коде. Давайте посмотрим, как можно улучшить написанный ранее класс.
Свойства
1. Пример из класса:
public Uri URL { set { _url = value; } }
Данный код отвечает за установку ссылки из внешнего кода т.е. свойство (вспоминаем аналогию с Delphi). Здесь мне посоветовали следующее, цитирую:
Write-only свойства — плохая практика. Лучше заменить на метод вида SetUrl()
Таким образом, если свойство используется только на приемку параметров, лучше преобразовать его в метод:
public void SetUrl(Uri url) { _url = url; }
События
1. Пример из класса:
public delegate void CategoryEventHandler(object sender, CategoryEventArgs e); public event CategoryEventHandler CategoryReceived;
Здесь мы создаем делегат для события, а потом объявляем само событие. Мне указали, что это лишнее и можно использовать обобщенный делегат EventHandler<>:
public event EventHandler<CategoryEventArgs> CategoryReceived;
2. Пример из класса:
if (ThreadStarted != null) { ThreadStarted(this, new ThreadEventArgs(catThread)); }
Этот код используется для генерации события из класса, для того чтобы внешний код мог его обработать. Мне рекомендовали заменить вызов события виртуальным protected методом, чтобы класс-наследник мог влиять на логику генерации события:
protected virtual void OnThreadStarted(object sender, Thread catThread) { if (ThreadStarted != null) { ThreadStarted(this, new ThreadEventArgs(catThread)); } }
После чего в коде класса можно будет указать компактный вызов метода (события):
OnThreadStarted(this, Thread.CurrentThread);
А если пользователю потребуется, он сможет переопределить данное событие в своем классе, которое должно наследовать LiveInternet класс.
Деструкторы
1. Пример из класса:
HttpWebResponse webResponse = (HttpWebResponse)webRequest.GetResponse(); Stream streamResponse = webResponse.GetResponseStream(); StreamReader readerResponse = new StreamReader(streamResponse); ... webResponse.Close(); streamResponse.Close(); readerResponse.Close();
В этом примере мы создаем и уничтожаем экземпляры классов, которые реализуют интерфейс IDisposable. Поэтому в данном случае можно применить второе назначение директивы using:
using (HttpWebResponse webResponse = (HttpWebResponse)webRequest.GetResponse()) { ... using (Stream streamResponse = webResponse.GetResponseStream()) { ... using (StreamReader readerResponse = new StreamReader(streamResponse)) { ... } // здесь уничтожается readerResponse ... } // здесь уничтожается streamResponse ... } // здесь уничтожается webResponse
Синхронизация
1. Пример из класса:
lock (this) { ... }
Здесь мы блокируем объект this для выполнения кода внутри в потокобезопасном режиме. Так как согласно MSDN:
lock (this) может привести к проблеме, если к экземпляру допускается открытый доступ
Поэтому лучше объявить отдельный объект и блокировать его:
private object lockThread; lock (lockThread) { ... }
Также мне рекомендовали вместо явного создания потоков использовать асинхронные методы класса WebRequest — BeginGetResponse/EndGetResponse.