Tips n Tricks
Atom Ответить
19.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#. Он показывает допущенные ошибки и предупреждает заранее о неправильном коде.



Теги:


Спасибо:



Скидка 15% на все продукты до 5 апреля (осталось 5 дней).

15 Ответов
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))
    Автор топика
    Спасибо:

    Mikhail Sukhov

    Фотография
    Автор статей Программист Трейдер
    Дата: 07.05.2011
    Ответить


    Подниму топик парочкой советов:


    1. internal на методы, свойста и поля (но не типа) в C# признак плохого дизайна. Говорит о том, что приосходит размытость границ логики.
    2. Код
      Trace.WriteLineIf(TracingLevel > 2, record.GetStrings().Join(";"));
      Плох тем, что не важно, есть необходимый уровень трассировки или нет, строчка будет собираться всегда. Если такая конструкция встречается в коде, который вызывается часто, может снизить производительность.
    3. Любите readonly. Тоесть, когда создаете поля, сразу их делайте readonly. Снять атрибут можно всегда. Но он предотвратит от нежелательной логики перетирания значения.
    4. Два кода абсолютно эквивалентны:
      Код
      public class RootClass
      {
      private class NestedClass
      {
      internal int Prop { get; set; }
      }
      }


      Код
      public class RootClass
      {
      private class NestedClass
      {
      public int Prop { get; set; }
      }
      }

    Автор топика
    Спасибо:

    aspirant

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


    Mikhail Sukhov Перейти
    Подниму топик парочкой советов:


    1. internal на методы, свойста и поля (но не типа) в C# признак плохого дизайна. Говорит о том, что приосходит размытость границ логики.
    2. Код
      Trace.WriteLineIf(TracingLevel > 2, record.GetStrings().Join(";"));
      Плох тем, что не важно, есть необходимый уровень трассировки или нет, строчка будет собираться всегда. Если такая конструкция встречается в коде, который вызывается часто, может снизить производительность.


    Сделал криво, знаю. Уже думал над тем, чтобы переделать + поменять internal на public. В релиз не будет попадать вообще: выделю в отдельный метод с аттрибутом [Conditional("DEBUG")]
    Спасибо:

    anothar

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


    Добрый день. Если верить http://www.codeproject.com/KB/cs/WeakEvents.aspx, кот. ссылается на Ecma cпецификацию, то для обеспечения потокобезопасности делегатов необходимо еще и локирование:
    Код

    EventHandler eh;
    lock (this) { eh = MyEvent; }
    if (eh != null) eh(this, EventArgs.Empty);
    Спасибо:

    aspirant

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


    @esper, может выставишь у себя tab'ы (Tools \ Options \ Text Editor \ Tab \ Keep tabs)?
    Спасибо:

    esper

    Фотография
    Программист
    Дата: 28.05.2011
    Ответить


    aspirant Перейти
    @esper, может выставишь у себя tab'ы (Tools \ Options \ Text Editor \ Tab \ Keep tabs)?

    СделалSmile
    Спасибо:


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

    loading
    clippy