Tips n Tricks. StockSharp

Tips n Tricks

20.01.2011

Mikhail Sukhov

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

Code
foreach (KeyValuePair<int, List<string>> pair in _sections)

Тут лучше использовать ключевое слово var
Code
foreach (var pair in _sections)
Code
String.Join(Environment.NewLine, pair.Value.ToArray())

С помощью Ecng.Common пишется короче
Code
pair.Value.Join(Environment.NewLine)
Code
protected List<string> _sectionNames;

Тут или область видимости должна быть private или название должно быть SectionName.
Code
String.Format("{0}={1}", key, value)

С помощью Ecng.Common пишется короче
Code
"{0}={1}".Put(key, value)
Code
_sections[_sectionNames.IndexOf(sectionName)];

Лучше проверить через Contains.
Code
try
{
....
}
catch (KeyNotFoundException ex)
{
throw new KeyNotFoundException(String.Format("Неопознанный раздел конфиг-файла: {0}", sectionName), ex);
}

Старайтесь минимизироваться перехват исключений. В том случае это решается через простую проверку на существование в коллекции.
Code
throw new InvalidOperationException(String.Format("Ключ {0} не найден", key));

Я бы заменил на ArgumentException. Он как то более осмысленный.
Не пишите Decimal, Int64, String или Single. Понятно, что это межъязыковое название. Но лучше использовать то, что родное для C#.
Code
public const int DEFAULT_PLAZA2_PORT = 4001;

Стиль именования как в Win32 API. Последний умер де факто много лет назад, наследие его живет и по сей день.
Добавляйте xml комментарии к самим классам, а не только его членам (видел в некоторых местах через // что есть совсем не то). И не забывайте про русский язык. В конце предложения ставится точка.
Пользуйтесь R#. Он показывает допущенные ошибки и предупреждает заранее о неправильном коде.

Теги:

Plaza 2

Спасибо:

Инструмент анализа - первый шаг к профиту.

1 2 >

aspirant

Дата: 20.01.2011

Mikhail Sukhov

Code

_sections[_sectionNames.IndexOf(sectionName)];

Лучше проверить через Contains.

Code

try
{
....
}
catch (KeyNotFoundException ex)
{
throw new KeyNotFoundException(String.Format("Неопознанный раздел конфиг-файла: {0}", sectionName), ex);
}

Старайтесь минимизироваться перехват исключений. В том случае это решается через простую проверку на существование в коллекции.

Большинство комментариев относится ко мне. Все поправил за исключением верхнего куска. Вот мое мнение: давно на каком-то MS'ском блоге читал рекомендацию: если в большинстве случаев ключи будут находиться, лучше сразу возвращать значение через метод this[TKey key] и перехватывать неопознанные ключи через исключение. Если вероятность исключения велика, лучше это делать через метод Contains. Это кусок кода написан в protected методе, к которому обращается только мой плазовский код. Входящие значения ключей заранее известны и контролируются, т.е. вероятность ненахождения ключей мала.

Михаил, за вами, как за менеджером проекта, последнее слово.

Спасибо:

Mikhail Sukhov

Дата: 20.01.2011

aspirant

Большинство комментариев относится ко мне.

Потому что другие пока код с логикой не писали. Вот сейчас начнут, и будут комментарии к другим.

aspirant

Mikhail Sukhov

Code

_sections[_sectionNames.IndexOf(sectionName)];

Лучше проверить через Contains.

Code

try
{
....
}
catch (KeyNotFoundException ex)
{
throw new KeyNotFoundException(String.Format("Неопознанный раздел конфиг-файла: {0}", sectionName), ex);
}

Вот мое мнение: давно на каком-то MS'ском блоге читал рекомендацию: если в большинстве случаев ключи будут находиться, лучше сразу возвращать значение через метод this[TKey key] и перехватывать неопознанные ключи через исключение. Если вероятность исключения велика, лучше это делать через метод Contains.

Тут не столько дело в перехвате и в Contains, сколько в сложности конструкции

Code

_sections[_sectionNames.IndexOf(sectionName)];

Еще раз посмотрел. А почем бы сразу было не сделать Dictionary?

aspirant

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

Насчет модификаторов доступа. Есть подозрение что они неправильно используются. Можете привести их описание применительно к случаям использования - когда какой использовать?

Спасибо:

aspirant

Дата: 20.01.2011

Mikhail Sukhov

Тут не столько дело в перехвате и в Contains, сколько в сложности конструкции

Code

_sections[_sectionNames.IndexOf(sectionName)];

Еще раз посмотрел. А почем бы сразу было не сделать Dictionary?

Вы правы: что я действительно здесь намудрил. Сегодня сведу все в Dictionary.

Mikhail Sukhov

aspirant

protected может использоваться только классами-наследниками, ну и самим классом тоже. Снаружи он не виден. Я имел в виду, что у меня есть класс ConfigParser. От него наследует класс ClientRouterConfigParser, который и запрашивает информацию, передавая ключи.

Спасибо:

Mikhail Sukhov

Дата: 20.01.2011

aspirant

Все ок. Я не увидел наследования. Но все равно, на будущее. Стиль именования протектем полей такой же как и паблик. Потому как смысл у них почти одинаковый.

Спасибо:

aspirant

Дата: 21.01.2011

aspirant

Mikhail Sukhov

Тут не столько дело в перехвате и в Contains, сколько в сложности конструкции

Code

_sections[_sectionNames.IndexOf(sectionName)];

Еще раз посмотрел. А почем бы сразу было не сделать Dictionary?

Вы правы: что я действительно здесь намудрил. Сегодня сведу все в Dictionary.

Только что закоммитил исправленный файл с классом ConfigParser. Идея такова: это базовый класс, который умеет парсить конфиг-файлы в формате Плазы. Для каждого конфиг-файла создается класс-наследник, который в своем конструкторе заполняет массив SectionNames списком возможных ключей для этого файла. Пока я только создал класс ClientRouterConfigParser, который настраивает главный конфиг-файл роутера Плазы client_router.ini. Я пока не разобрался, нужно ли будет настраивать другие конфиги.

Спасибо:

Mikhail Sukhov

Дата: 21.01.2011

aspirant

Это хорошо. Только зря вы убрали метод FindSection. Вообще лучше по максимуму переменные делать private и доступ к ним или через методы или через свойства. Лучше через методы, чтобы базовый класс делал базовую работу, а не просто отдавал свое состояние дочерним.

Спасибо:

Mikhail Sukhov

Дата: 21.01.2011

Еще список того, что может пригодиться:

Code
var myNumber = Convert.ToInt32(myString);

можно писать короче:
Code
var myNumber = myString.To<int>();
Code
if (string.IsNullOrEmpty(arg))

можно писать короче:
Code
if (arg.IsEmpty())

Спасибо:

Mikhail Sukhov

Дата: 21.01.2011

Code

_dataStream.StreamLifeNumChanged += new IP2DataStreamEvents_StreamLifeNumChangedEventHandler(OnStreamLifeNumChanged);

можно писать короче:

Code

v_dataStream.StreamLifeNumChanged += OnStreamLifeNumChanged;

Спасибо:

Mikhail Sukhov

Дата: 30.01.2011

Code

if (MyEvent != null)
MyEvent();

Проверка на null - это правильно. Но на следующей строчке MyEvent может стать null (в другом потоке произошла отписка от события) и будет NullReferenceException. Решается через доп переменную:

Code

var evt = MyEvent;
if (evt != null)
evt();

Если не создавать собственных делегатов (видимо равнение идет на Плазу, где они создаются автоматически, а не потому что так правильно), то запись будет короче:

Code

MyEvent.SafeInvoke();

Спасибо:

Mikhail Sukhov

Дата: 18.03.2011

int и Int32 - это алиасы. так же для bool и Boolean и т.д. Выражение вида:

Code

if (type == typeof(bool) || type == typeof(Boolean))

эквивалентно:

Code

if (type == typeof(bool))

Спасибо: