Tips n Tricks
Atom
20.01.2011


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


  1. Код
    foreach (KeyValuePair<int, List<string>> pair in _sections)

    Тут лучше использовать ключевое слово var
    Код
    foreach (var pair in _sections)

  2. Код
    String.Join(Environment.NewLine, pair.Value.ToArray())

    С помощью Ecng.Common пишется короче
    Код
    pair.Value.Join(Environment.NewLine)

  3. Код
    protected List<string> _sectionNames;

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

    С помощью Ecng.Common пишется короче
    Код
    "{0}={1}".Put(key, value)

  5. Код
    _sections[_sectionNames.IndexOf(sectionName)];

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

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

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

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



Теги:


Спасибо:


1 2  >
aspirant

Фотография
Дата: 20.01.2011
Ответить


Mikhail Sukhov Перейти

  • Код
    _sections[_sectionNames.IndexOf(sectionName)];

    Лучше проверить через Contains.
  • Код
    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 Перейти

  • Код
    _sections[_sectionNames.IndexOf(sectionName)];

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

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


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


    Тут не столько дело в перехвате и в Contains, сколько в сложности конструкции
    Код
    _sections[_sectionNames.IndexOf(sectionName)];

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

    aspirant Перейти

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


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

    aspirant

    Фотография
    Дата: 20.01.2011
    Ответить


    Mikhail Sukhov Перейти


    Тут не столько дело в перехвате и в Contains, сколько в сложности конструкции
    Код
    _sections[_sectionNames.IndexOf(sectionName)];

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



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

    Mikhail Sukhov Перейти


    aspirant Перейти

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


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


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

    Mikhail Sukhov

    Фотография
    Дата: 20.01.2011
    Ответить


    aspirant Перейти

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


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

    aspirant

    Фотография
    Дата: 21.01.2011
    Ответить


    aspirant Перейти
    Mikhail Sukhov Перейти


    Тут не столько дело в перехвате и в Contains, сколько в сложности конструкции
    Код
    _sections[_sectionNames.IndexOf(sectionName)];

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



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



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

    Mikhail Sukhov

    Фотография
    Дата: 21.01.2011
    Ответить


    aspirant Перейти

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


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

    Mikhail Sukhov

    Фотография
    Дата: 21.01.2011
    Ответить


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


    1. Код
      var myNumber = Convert.ToInt32(myString);

      можно писать короче:
      Код
      var myNumber = myString.To<int>();

    2. Код
      if (string.IsNullOrEmpty(arg))

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

    Спасибо:

    Mikhail Sukhov

    Фотография
    Дата: 21.01.2011
    Ответить


    Код
    _dataStream.StreamLifeNumChanged += new IP2DataStreamEvents_StreamLifeNumChangedEventHandler(OnStreamLifeNumChanged);

    можно писать короче:
    Код
    v_dataStream.StreamLifeNumChanged += OnStreamLifeNumChanged;
    Спасибо:

    Mikhail Sukhov

    Фотография
    Дата: 30.01.2011
    Ответить


    Код
    if (MyEvent != null)
        MyEvent();


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

    Код
    var evt = MyEvent;
    if (evt != null)
        evt();


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

    Код
    MyEvent.SafeInvoke();
    Спасибо:

    Mikhail Sukhov

    Фотография
    Дата: 18.03.2011
    Ответить


    int и Int32 - это алиасы. так же для bool и Boolean и т.д. Выражение вида:
    Код
    if (type == typeof(bool) || type == typeof(Boolean))

    эквивалентно:
    Код
    if (type == typeof(bool))
    Спасибо:
    1 2  >

    Добавить файлы через драг-н-дроп, , или вставить из буфера обмена.

    loading
    clippy