Привет.

Зачем ты навставлял проверок такого вида:

if (String.IsNullOrEmpty(filePath))
{
    return false;
}
...
            if (request == null)
{
    return;
}

?

Там действительно приходил null и пустая строка? Если, да, то неужели это нормальное поведение этих методов?

Если на оба последних вопроса ответ отрицательный, и этот код создан для чистоты, так сказать, то будь добр, замени его на генерацию исключений ArgumentNullException/ArgumentException.

Сразу объясняю почему так нельзя делать. Рано или поздно кто-то может ошибиться и случаяно передать в такой метод null или пустую строку. Наличие исключения или даже просто отсуствие проверки параметров приведет к тому, что мы увидим эту ошибку. А вот возврат null-ов и т.п. приведет к появлению исключений в совсем дургм месте или просто замажет ошибку (будет некорректное поведение о котором никто не будет знать).

ЗЫ

Да, ив if-ов с одним выражением внутри не стоит ставить скобки. Они соврешенно лишние. Только место занимают. Отбивки достаточно.

Здравствуйте, VladD2, Вы писали:

VD>Зачем ты навставлял проверок такого вида:
VD>

VD>if (String.IsNullOrEmpty(filePath))
VD>{
VD>    return false;
VD>}
VD>...
VD>            if (request == null)
VD>{
VD>    return;
VD>}
VD>

VD>?

VD>Там действительно приходил null и пустая строка? Если, да, то неужели это нормальное поведение этих методов?

В некоторых случаях там действительно может прийти null. Может прийти и пустая строка. Use-case сценарий приводить не буду, но пару раз ловил исключения при тестировании. Проверки я ставил только там, где есть смысл.

VD>Если на оба последних вопроса ответ отрицательный, и этот код создан для чистоты, так сказать, то будь добр, замени его на генерацию исключений ArgumentNullException/ArgumentException.

Хм, может лучше ассерты?

VD>Сразу объясняю почему так нельзя делать. Рано или поздно кто-то может ошибиться и случаяно передать в такой метод null или пустую строку. Наличие исключения или даже просто отсуствие проверки параметров приведет к тому, что мы увидим эту ошибку. А вот возврат null-ов и т.п. приведет к появлению исключений в совсем дургм месте или просто замажет ошибку (будет некорректное поведение о котором никто не будет знать).

Если разработчик точно уверен, что в этом методе не придет null, тогда он ставит Debug.Assert. Я так глубоко не копал, чтобы точно знать, какие данные передаются в методы. Поиск по вызывающим функциям показал, что вполне могут приходить некорректные строки. Возврат null-а я вообще нигде не делаю, и больно бью по рукам своих сотрудников если они так делают.

VD>Да, ив if-ов с одним выражением внутри не стоит ставить скобки. Они соврешенно лишние. Только место занимают. Отбивки достаточно.

Ага, и в итоге блок if-a вообще становится незаметным в общем объеме кода. Странно, почему вы тогда не используете форматирование K&R (фигурная скобка ставится в конце строки) — ведь это еще больше места экономит? С другой стороны, если так сильно хочется, то не буду занимать места — как скажешь, так и буду делать.

P.S. Все-таки не понимаю, к чему придирки к проверкам? Сами же сказали, что код находится на еще очень ранней стадии. Я отловил пару раз NullReferenceException и поставил проверки. Я еще не настолько разобрался в проекте, чтобы знать корень этого null-а, и поставил проверки, чтобы просто студия не вываливалась. Предложи мне другую стратегию разработки, и тогда я не буду так делать.

Здравствуйте, Nuald, Вы писали:

N>В некоторых случаях там действительно может прийти null. Может прийти и пустая строка. Use-case сценарий приводить не буду, но пару раз ловил исключения при тестировании. Проверки я ставил только там, где есть смысл.

Не, не. Надо четко понимать зачем там приходит null. По логике этих методов вроде бы никакие пустые строки там быть не должны.

VD>>Если на оба последних вопроса ответ отрицательный, и этот код создан для чистоты, так сказать, то будь добр, замени его на генерацию исключений ArgumentNullException/ArgumentException.

N>Хм, может лучше ассерты?

Нет, лучше исключения. Что программа будет делать после того как этот ассерт закроется? Как продолжить работу в этой ситуации? Ведь на null и пустые строки там рассчета нет.

N>Если разработчик точно уверен, что в этом методе не придет null, тогда он ставит Debug.Assert.

Нет, см. выше. Конечно все проверки — это вообще сфера предпочтений и стратегий, но у нас принята стратегия в которой Assert и темболее Debug... ставится для того чтобы проверить свои предполжения. А когда ты пишишь метод который может быть вызван черт знает откуда, черт знает кем, то неверные входные условия являются исключительной ситуацией для функции, а значит должны генерировать исключения.

Исключения выгдны еще и тем, что в релизе они позволят востановиться до работоспосбного состояния. Ассерты же будут только досождать пользователю своими окнами. За ними будут появляться исключения, только уже менее информативные.

N> Я так глубоко не копал, чтобы точно знать, какие данные передаются в методы.

Темболее странно в таких условиях лепить if-ы замазывающие аварийные ситуации.

N> Поиск по вызывающим функциям показал, что вполне могут приходить некорректные строки.

Конкретнее пожалуйста. Что значит "некорретные"? И в следствии чего они могут приходить.

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

N>Ага, и в итоге блок if-a вообще становится незаметным в общем объеме кода.

Понимаеш ли в чем дело. Это не обсуждается. Это правила форматирования принятые в проекте. Фигурные скобки ставятся тольк для случаев когда есть более одного statment-а и возможно когда есть многострочное условие if-а и его тело плохо различимо.

N> Странно, почему вы тогда не используете форматирование K&R

Потому что по принятому у нас стилю оформления кода. Фигурные скобки ставятся всятся на новой строке.

N>P.S. Все-таки не понимаю, к чему придирки к проверкам?

Это очень плохо что не понимаешь. Я попытался обяснить. Посторайся понять. Основная причина — такие проекри могу скрыть реальные ошибки и превратить их из ошибок приводящих к исключению (кторое легко отслеживается) к ошибкам в логике работы программы (которые ловятся и исправляются намного сложнее).

N> Сами же сказали, что код находится на еще очень ранней стадии.

Это не повод писать код от балды.

N> Я отловил пару раз NullReferenceException и поставил проверки.

NullReferenceException не повод замазывать ошибку. Он скорее всего является следсвием другой ошибки. Вот ее и надо устранять. А проверки на null не должны быть связаны с тем что где-то там есть NullReferenceException. Они должны быть частью логики работы программы. Поэтому я и спрашиваю в чем эта логика. Так как мне кажется, что в некоторых местах эта логика нарушена или как минимум усложнена.

ОК, если не понимаешь, давай разберем конкретные случаи:

if (!String.IsNullOrEmpty(location))
    _projectLocation = location;
else
    _projectLocation = Path.GetDirectoryName(fileName);

Здесь можно сказать все ОК. Только зачем делать отрицающее условие? Оно усложняет чтение. В таких случаях нужно писать:

if (String.IsNullOrEmpty(location))
    _projectLocation = Path.GetDirectoryName(fileName);
else
    _projectLocation = location;

Отрицающие условия хуже читаются и в if/else и ?: их лучше не использовать.

public bool IsFileInProject(string filePath)
{
    if (String.IsNullOrEmpty(filePath))
    {
        return false;
    }
    return _filesMap.ContainsKey(filePath);
}

Вот здесь уже большой вопрос. Я не представляю себе шататной ситуации когда путь к файлу будет пустым. Хотя бы имя то у него должно быть? А раз так тут надо вставлять проверки с возбуждением исключеий. Или хотя бы ничего не делать.

Заметь, даже если не ставить проверки, то функция ContainsKey() класса Dictionary проверит аргументи и выдаст исключение ArgumentNullException. Твой же код скроет ошибку.

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

public void UpdateFile(ParseRequest request)
{
    if (request == null)
    {
        return;
    }
    int    oldTimestamp;
    string filePath  = request.FileName;
    int    timestamp = request.Timestamp;

    if (!_filesMap.TryGetValue(filePath, out oldTimestamp))
        throw new ArgumentException("File '" + filePath + "' not found.", "filePath");

    if (timestamp != oldTimestamp)
    {
        Engine.Sources.AddOrUpdate(filePath, GetFileContent(request));
        _filesMap[filePath] = timestamp;
    }
}

А вот здесь на лицо явное замазывание ошибки. Если где-то произойдет ошибка и в request окажется null, то мы не узнаем о ней, так как твоя проверка скроет ошибку.

string GetFileContent(ParseRequest request)
{
    if (request == null)
    {
        return String.Empty;
    }
    string text = request.Text;

    if (String.IsNullOrEmpty(text))
        text = File.ReadAllText(request.FileName);

    return text;
}

Еще хуже! Здесь мы уже наверняка получим неверное поведение. Зачем нам пустая строка? null в request является ошибкой!

private static NemerleFileNodeProperties GetNodeProperties(IVsHierarchy hierarchy, uint itemID)
{
    if (hierarchy == null)
    {
        return new NemerleFileNodeProperties(null);
    }
    object propertyValue;
    int    hr = hierarchy.GetProperty(itemID, (int)__VSHPROPID.VSHPROPID_BrowseObject, out propertyValue);

    if (hr != VSConstants.S_OK)
        throw new ArgumentException("Can't obtain VSHPROPID_BrowseObject for item with ID " + itemID, "itemID");

    FileNodeProperties properties = propertyValue as FileNodeProperties;
    if (properties != null)
    {
        return new NemerleFileNodeProperties(properties.Node);
    }
    return new NemerleFileNodeProperties(null);
}

Тут не ясно. На лицо изменение поведения метода, но есть ли в этом смысл? Или это очередная замазка другой проблемы?

public static ProjectInfo FindProject(string fileName)
{
    if (String.IsNullOrEmpty(fileName))
    {
        return null;
    }
    foreach (ProjectInfo proj in _projectsInfos)
        if (proj.IsFileInProject(fileName))
            return proj;

    return null;
}

Здесь та же ситуация что и с IsFileInProject(). Вместо диагностики ошибки мы получим ее замазывание и будем часами искать ошибку.

N> Я еще не настолько разобрался в проекте, чтобы знать корень этого null-а, и поставил проверки, чтобы просто студия не вываливалась.

Ставить проверки чтобы что-то не вываливалось категорически не надо! Если ты не в силах понять причину проблемы, то просто опиши ее в этом форуме или добавь описание в баг-трекер. При этом обясни как воспроизвести эту ошибку. И тогда мы уже вместе решим что с ней делать.

N> Предложи мне другую стратегию разработки, и тогда я не буду так делать.

Стратегия проста. Баги надо устранять, а не замазывать. Любая провекра должна делаться обоснованно. Фраза "чтобы исключение не вылетала" не может быть обоснованием.

Сейчас у нас много ошибок, так как проект находится в стадии разработки. И нам очень важно, чтобы ошибки были как можно более легко выявляемы.

Здравствуйте, Nuald, Вы писали:

Свежий пример:

protected override AssemblyReferenceNode CreateAssemblyReferenceNode(ProjectElement element)
{
    // VladD2:
    // ReferenceContainerNode does not support reference to files (instead of
    // get assembly name from file it tries to use file name as assembly name
    // (via System.Reflection.AssemblyName()).

    string item = element.Item.FinalItemSpec;
    NemerleAssemblyReferenceNode node = null;

    try
    {
        if (File.Exists(item))
            node = new NemerleAssemblyReferenceNode(ProjectMgr, item);
        else
            node = new NemerleAssemblyReferenceNode(ProjectMgr, element);
    }
    catch (ArgumentNullException e)
    {
        Trace.WriteLine("Exception : " + e.Message);
    }
    catch (FileNotFoundException e)
    {
        Trace.WriteLine("Exception : " + e.Message);
    }
    catch (BadImageFormatException e)
    {
        Trace.WriteLine("Exception : " + e.Message);
    }
    catch (FileLoadException e)
    {
        Trace.WriteLine("Exception : " + e.Message);
    }
    catch (System.Security.SecurityException e)
    {
        Trace.WriteLine("Exception : " + e.Message);
    }

    return node;
}

Можно мне объяснить суть этого решения?

Зачем информацию о том, что файл не найдет выводить в отладочную консоль? Неужели пользователь не достоин знать о ней?

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

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

Здравствуйте, Nuald, Вы писали:

Да, писать проверки в обратную сторону тоже не следует:

null == _assemblyRef

Чувствуется что ты недавно из мира С++ где подобные трюки были нормой в следствии дырявости языка. Но в C# и Nemerle проблем подобных С++-ных мнет. А читабельность кода от такой записи страдает. Так что писать нужно:

_assemblyRef == null

Стиль кода в проекте должен быть единым. Нет ничего хуже чем читать код написанный в разных стилях.

VD>Стиль кода в проекте должен быть единым. Нет ничего хуже чем читать код написанный в разных стилях.

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

например, чувак, пишущий if (0 == foo) ... явно читал книгу по с++ для начинающих (Дейтел, да? )

чувак, который пишет for (int i = 0; i < n; ++i) ... либо умен достаточно, чтобы понимать разницу в пре- и пост- нотации
либо (если это он на с#-е пишет) пришел из мира с++, и не понимает, скорее всего, разницу между языками
а я вот на c# пишу тоже в нотации ++i, но это чтобы выпендрится и показать, что я не из бейсика в дотнет пришел
но в любом случае тот, кто пишет ++i по привычке — маниак в хорошем смысле

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

ну и наконец, берешь чужой проект — а в нем нотация "неправильная", и написан без знания "best practices"
опять же переделываешь потихоньку, а что переделано, отмечаешь своим стилем (типа правильным)
ну, это если сам работаешь или с 1-2 человеками

Здравствуйте, VladD2, Вы писали:

VD>Да, писать проверки в обратную сторону тоже не следует:
VD>

VD>null == _assemblyRef
VD>

VD>Чувствуется что ты недавно из мира С++ где подобные трюки были нормой в следствии дырявости языка. Но в C# и Nemerle проблем подобных С++-ных мнет. А читабельность кода от такой записи страдает. Так что писать нужно:

Неправильно чувствуется. Это я скопировал из ReferenceContainerNode, и не стал править. Если это относится к стилям кода, то впредь буду править.

VD>

VD>_assemblyRef == null
VD>

VD>Стиль кода в проекте должен быть единым. Нет ничего хуже чем читать код написанный в разных стилях.

Могу тебя понять, но с другой стороны, когда работаешь в проектах, пишушихся несколькими командами, начинаешь снисходительно относится к таким мелочам, чтобы из-за них к кому-то придираться или править код. Впрочем, я уже выше сказал — впредь буду исправлять такие проверки.

Здравствуйте, VladD2, Вы писали:

VD>Можно мне объяснить суть этого решения?

Могу. Я убрал вызов base.CreateAssemblyReferenceNode и чтобы максимально передать его функциональность, взял код из него и вставил в наш класс.

VD>Зачем информацию о том, что файл не найдет выводить в отладочную консоль? Неужели пользователь не достоин знать о ней?

Вопрос не ко мне, а к разработчикам VS SDK.

VD>А зачем обрабатвать ArgumentNullException? Неужели это штатная ситуация? Подобную обработку я могу объяснить только наличием косяка в библиотеке, но такие вещи обязательно нужно описывать в коментариях.

Вопрос не ко мне, а к разработчикам VS SDK. Они в комментариях ничего не описали.

VD>В общем, мне кажется, что ждесь ArgumentNullException нужно вообще удалить, а на остальные исключения выдавать не Trace, а диалоговое окно с сообщением об ошибке.

Хм, а у нас есть какой-нить стандарт на вывод такого рода диалоговых окон, или вспомогательные классы, делающие это? Неохота выводить окна в разных стилях.

Здравствуйте, VladD2, Вы писали:

VD>Нет, см. выше. Конечно все проверки — это вообще сфера предпочтений и стратегий, но у нас принята стратегия в которой Assert и темболее Debug... ставится для того чтобы проверить свои предполжения. А когда ты пишишь метод который может быть вызван черт знает откуда, черт знает кем, то неверные входные условия являются исключительной ситуацией для функции, а значит должны генерировать исключения.

Хорошо, я не знал, что у вас используется такая стратегия. Буду впредь придерживаться того же.

VD>Конкретнее пожалуйста. Что значит "некорретные"? И в следствии чего они могут приходить.

Пример. Переименовываешь файл, нажимаешь Del, стирается полное имя файла, вылетает необработанное исключение, студия закрывается с ошибкой. Какое здесь должно быть поведение? Если придерживаться стратегии, которую ты описал, так и должно быть. Меня, как конечного пользователя, это сильно напрягает. Как разработчик, причину я устранить не могу, но замазыванием я спасаю студию, пусть и с вылетом где-то First-chance exception. И где компромисс? Ситуация чисто теоретическая, я просто хочу знать, как в ней быть, чтобы впредь ничего не сломать.

N>> Я отловил пару раз NullReferenceException и поставил проверки.

VD>NullReferenceException не повод замазывать ошибку. Он скорее всего является следсвием другой ошибки. Вот ее и надо устранять. А проверки на null не должны быть связаны с тем что где-то там есть NullReferenceException. Они должны быть частью логики работы программы. Поэтому я и спрашиваю в чем эта логика. Так как мне кажется, что в некоторых местах эта логика нарушена или как минимум усложнена.

Опять привожу для примера ситуацию выше. Замазыванием ошибки я сделал работу конечного пользователя более комфортной. В чем я неправ? Да, я не устранил причину, но до некоторых причин докопаться очень тяжело или вообще нереально, т.к. они вызываются где-то из недр студии.

VD>Здесь можно сказать все ОК. Только зачем делать отрицающее условие? Оно усложняет чтение. В таких случаях нужно писать:
VD>

VD>if (String.IsNullOrEmpty(location))
VD>    _projectLocation = Path.GetDirectoryName(fileName);
VD>else
VD>    _projectLocation = location;
VD>

VD>Отрицающие условия хуже читаются и в if/else и ?: их лучше не использовать.

Хорошо. Так впредь и буду делать.

VD>

VD>public bool IsFileInProject(string filePath)
VD>{
VD>    if (String.IsNullOrEmpty(filePath))
VD>    {
VD>        return false;
VD>    }
VD>    return _filesMap.ContainsKey(filePath);
VD>}
VD>

VD>Вот здесь уже большой вопрос. Я не представляю себе шататной ситуации когда путь к файлу будет пустым. Хотя бы имя то у него должно быть? А раз так тут надо вставлять проверки с возбуждением исключеий. Или хотя бы ничего не делать.

Ситуацию я описал выше.

VD>Заметь, даже если не ставить проверки, то функция ContainsKey() класса Dictionary проверит аргументи и выдаст исключение ArgumentNullException. Твой же код скроет ошибку.

ArgumentNullException вывалит студию.

VD>

VD>private static NemerleFileNodeProperties GetNodeProperties(IVsHierarchy hierarchy, uint itemID)
VD>{
VD>    if (hierarchy == null)
VD>    {
VD>        return new NemerleFileNodeProperties(null);
VD>    }
VD>    object propertyValue;
VD>    int    hr = hierarchy.GetProperty(itemID, (int)__VSHPROPID.VSHPROPID_BrowseObject, out propertyValue);

VD>    if (hr != VSConstants.S_OK)
VD>        throw new ArgumentException("Can't obtain VSHPROPID_BrowseObject for item with ID " + itemID, "itemID");

VD>    FileNodeProperties properties = propertyValue as FileNodeProperties;
VD>    if (properties != null)
VD>    {
VD>        return new NemerleFileNodeProperties(properties.Node);
VD>    }
VD>    return new NemerleFileNodeProperties(null);
VD>}
VD>

VD>Тут не ясно. На лицо изменение поведения метода, но есть ли в этом смысл? Или это очередная замазка другой проблемы?

Нет, не замазка. Смысл есть. Единственное, не уверен насчет new NemerleFileNodeProperties(null). Здесь надо кидать исключение, но не знаю какое.

VD>Ставить проверки чтобы что-то не вываливалось категорически не надо! Если ты не в силах понять причину проблемы, то просто опиши ее в этом форуме или добавь описание в баг-трекер. При этом обясни как воспроизвести эту ошибку. И тогда мы уже вместе решим что с ней делать.

Ладно.

Здравствуйте, PhantomIvan, Вы писали:

PI>например, чувак, пишущий if (0 == foo) ... явно читал книгу по с++ для начинающих (Дейтел, да? )

Попиши на C++ хотя бы лет пять (не пожелал бы и врагу ), я на тебя посмотрю, как ты будешь условия писать. Впрочем, когда мы писали на C++, то обычно вводили булевскую переменную, и проверяли ее, а не писали условие в самом цикле. Компилятор этот вызов оптимизирует, а код становится более читабельным, т.к. обычно названием булевская переменная говорит, какой смысл несет то или иное условие.

PI>ну и наконец, берешь чужой проект — а в нем нотация "неправильная", и написан без знания "best practices"
PI>опять же переделываешь потихоньку, а что переделано, отмечаешь своим стилем (типа правильным)
PI>ну, это если сам работаешь или с 1-2 человеками

Завидую тебе. Проекты с 1-2 человеками

Здравствуйте, VladD2, Вы писали:

VD>В общем, мне кажется, что ждесь ArgumentNullException нужно вообще удалить, а на остальные исключения выдавать не Trace, а диалоговое окно с сообщением об ошибке.

Я бы вообще весь этот огород убрал и оставил бы один catch с выводом информации об ошибке.

Здравствуйте, VladD2, Вы писали:

VD>Да, ив if-ов с одним выражением внутри не стоит ставить скобки. Они соврешенно лишние. Только место занимают. Отбивки достаточно.

if(blaBlaBla) {
  выражение;
}//if

У меня другая точка зрения на этот вопрос, и вот почему: если в тело ифа понадобится добавить ещё одно выражение, то мне не придётся подниматься вверх ставить открывающую кавыку, спускаться вниз и ставить кавыку (то есть совершать в три раза больше действий). Ну и при удалении одного из выражений в многострочном ифе опять же придётся менять эти две строки (удалять кавыки).
Я ставлю кавыки всегда, вне зависимости от количествы выражений, так как оно позволяет наименее безболезненно для общего вида (и различных Merge-тулзов) менять код.

Так как ты употребил "не стОит", а не "нужно и требуется", то счёл возможным пофлеймить :о) Давно хотелось ;о)

Здравствуйте, Nuald, Вы писали:

N>Пример. Переименовываешь файл, нажимаешь Del, стирается полное имя файла, вылетает необработанное исключение, студия закрывается с ошибкой. Какое здесь должно быть поведение?

Только что повторил описанные тобой действия. При этом у меня уже измененный файл:
http://nemerle.org/svn/vs-plugin/trunk/Nemerle.VsIntegration/Project/ProjectInfo.cs
При попытке нажать Enter, если имя пустое появляется сообщение:

You must enter a name.

Так что все ОК. Возможно помогли правки по существу.

N> Если придерживаться стратегии, которую ты описал, так и должно быть. Меня, как конечного пользователя, это сильно напрягает.

Продукт не находится в стадии когда нужно говорить о конечных пользователях. Это раз.

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

У нас, например, удаление файлов вообще не работает. Ну, так надо содиться и разбираться.

N> Как разработчик, причину я устранить не могу, но замазыванием я спасаю студию, пусть и с вылетом где-то First-chance exception.

А в чем проблема?

N> И где компромисс? Ситуация чисто теоретическая, я просто хочу знать, как в ней быть, чтобы впредь ничего не сломать.

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

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

N>Опять привожу для примера ситуацию выше. Замазыванием ошибки я сделал работу конечного пользователя более комфортной.

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

Ты пользовался Дельфи? Вот во многих версиях ее IDE тоже было много заплаток. И переодически ее приходилось перезагружать, так как работать с ней было совершенно невозможно.

В общем, на начальных стадиях разрабокти, когда функциональность еще не доделана такие замазки очень опасны.

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

N> В чем я неправ? Да, я не устранил причину, но до некоторых причин докопаться очень тяжело или вообще нереально, т.к. они вызываются где-то из недр студии.

Если причина не ясна, то надо описать ее здесь или запостить в баг-трекер. Глядишь кто-то знает то что поможет решить эту проблему. Ну, или просто разберется более детально.

VD>>Вот здесь уже большой вопрос. Я не представляю себе шататной ситуации когда путь к файлу будет пустым. Хотя бы имя то у него должно быть? А раз так тут надо вставлять проверки с возбуждением исключеий. Или хотя бы ничего не делать.

N>Ситуацию я описал выше.

Вотя я заменил эту проверку на исключени и ровным счетом твоя ситуация не изменилась. Все работает. Значит проблема была в другом. А в следствии, например, глюков отладчика ты не смог ее обнаружить.

VD>>Заметь, даже если не ставить проверки, то функция ContainsKey() класса Dictionary проверит аргументи и выдаст исключение ArgumentNullException. Твой же код скроет ошибку.

N>ArgumentNullException вывалит студию.

А где он появляется?
Я пока не удалял твои обработчики, но поставил там точки прирывания и пока ни разу туда не попал.

Как воспроизвести эти исключения?

N>Нет, не замазка. Смысл есть. Единственное, не уверен насчет new NemerleFileNodeProperties(null). Здесь надо кидать исключение, но не знаю какое.

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

private static NemerleFileNodeProperties GetNodeProperties(IVsHierarchy hierarchy, uint itemID)
{
    ErrorHelper.ThrowIsNull(hierarchy, "hierarchy");

    object propertyValue;
    int    hr = hierarchy.GetProperty(itemID, (int)__VSHPROPID.VSHPROPID_BrowseObject, out propertyValue);

    if (hr != VSConstants.S_OK)
        throw new ArgumentException("Can't obtain VSHPROPID_BrowseObject for item with ID "
            + itemID, "itemID", new System.ComponentModel.Win32Exception(hr));

    NemerleFileNodeProperties properties = propertyValue as NemerleFileNodeProperties;

    if (properties == null)
        return new NemerleFileNodeProperties(((FileNodeProperties)propertyValue).Node);
    else
        return properties;
}

в рассчете на то, что свойства могут оказаться типа FileNodeProperties. Хотя это тоже шаманство, но хотя бы понятно что происходит.

VD>>Ставить проверки чтобы что-то не вываливалось категорически не надо! Если ты не в силах понять причину проблемы, то просто опиши ее в этом форуме или добавь описание в баг-трекер. При этом обясни как воспроизвести эту ошибку. И тогда мы уже вместе решим что с ней делать.

N>Ладно.

Ну, и славно.

Здравствуйте, PhantomIvan, Вы писали:

PI>не согласен
PI>когда проект "монстр", то любые зацепки к пониманию могут быть полезны
PI>и разница в стиле свидетельствует о том, что его писали разные девелоперы
PI>по таким остаточным следам можно иногда составить мнение о том или ином разработчике, и к некоторым частям кода относиться с "большим" доверием, чем к другим

PI>например, чувак, пишущий if (0 == foo) ... явно читал книгу по с++ для начинающих (Дейтел, да? )

А вот глумиться не надо. Я сам немало писал на C++ и прекрасно понимаю люде далющих так. Подобные ошибки очень неприяно искать. В C# конечно это полнешее излишество, но привычки есть привычки. От плохих привычек неспомненно нужно избавляться, но процесс этот не простой.

Здравствуйте, Nuald, Вы писали:

N>Попиши на C++ хотя бы лет пять (не пожелал бы и врагу ), я на тебя посмотрю, как ты будешь условия писать.

Я писал на С/С++ приблизительно 13 лет. И прекрасно понимаю твою привычку. Сам всех на плюсах заставлял так же делать. Но для C#/Nemerle — эта привычка лишняя и даже вредная. Я вот отучился.

Вообще уметь менять привычки и переключаться на другой стиль — это самая правильная привычка.

Здравствуйте, Nuald, Вы писали:

N>Неправильно чувствуется.

Тогда сори.

N> Это я скопировал из ReferenceContainerNode, и не стал править. Если это относится к стилям кода, то впредь буду править.

Код менеджед-оберток (MPF) писали настоящие индусы. С него пример точно лучше не брать. А по возможности его надо причесывать.

N>Могу тебя понять, но с другой стороны, когда работаешь в проектах, пишушихся несколькими командами,

Я тебя тоже могу понять. Вот проект компилятора пишется вообще в другом стиле. Там и пробелы в методах между именем и отывающей скобкой, и фигурные скобки в Кернихановском стиле, и много чего еще. Но это стиль проекта и приходится переключаться на другой стиль. В общем-то после некоторой практике привыкашь к этому и уже начинашь более спокойно относиться к разным стилям. Начинаешь понимать, что ценен не сам стиль, а его наличие . Раньше я тоже сильно воевал по поводу стилей. Но как раз работа над проектами с разным стилем очень помогла стань немного умнее.

N>начинаешь снисходительно относится к таким мелочам, чтобы из-за них к кому-то придираться или править код. Впрочем, я уже выше сказал — впредь буду исправлять такие проверки.

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

Здравствуйте, Nuald, Вы писали:

N>Здравствуйте, VladD2, Вы писали:

VD>>Можно мне объяснить суть этого решения?

N>Могу. Я убрал вызов base.CreateAssemblyReferenceNode и чтобы максимально передать его функциональность, взял код из него и вставил в наш класс.

Ясно. Код из интеграции крив. Живет он только на тотальном тестировании. Так что переносить его лучше творчески. Сохранять только суть.

VD>>Зачем информацию о том, что файл не найдет выводить в отладочную консоль? Неужели пользователь не достоин знать о ней?

N>Вопрос не ко мне, а к разработчикам VS SDK.

Ясно. Пока поставил туда брэйкпоинты. Будем глядеть когда это дело может вылететь. Далее наверно лучше вообще убрать эти проверки.

N>Вопрос не ко мне, а к разработчикам VS SDK. Они в комментариях ничего не описали.

Ясно.

N>Хм, а у нас есть какой-нить стандарт на вывод такого рода диалоговых окон, или вспомогательные классы, делающие это? Неохота выводить окна в разных стилях.

Кроме MessageBox.Show пока ничего нет. Но можно и завести.

PI>>например, чувак, пишущий if (0 == foo) ... явно читал книгу по с++ для начинающих (Дейтел, да? )

VD>А вот глумиться не надо. Я сам немало писал на C++ и прекрасно понимаю люде далющих так. Подобные ошибки очень неприяно искать. В C# конечно это полнешее излишество, но привычки есть привычки. От плохих привычек неспомненно нужно избавляться, но процесс этот не простой.

не, я не глумлюсь — просто шуткую чутка
когда-то насмотревшись хитрых условий типа while (++y = x++) ... , я относился всегда внимательно к этим местам
ошибившись 25 раз в if (expr == 0), на автомате уже писал правильно
то есть не меняя стиль, просто более внимательно проверял каждый if

я не спорю, что лучше
просто, что касается именно if-а, чисто эмпирическое наблюдение:
новички часто пишут if (0 == foo), потому что читали именно Дейтела
а те кто читали Страуструпа, пишут наоборот
конечно, есть профессионалы, которые пишут как Дейтел, Nuald например

но это все мелочи
вот outlining в плагине появился, заметил я
я сам не пользуюсь, но молодцы
количество переколбашенных при каждом обновлении с свн-а поражает
то есть молодцы, без шуток

Здравствуйте, _FRED_, Вы писали:

_FR>У меня другая точка зрения на этот вопрос,

Здорово, но в данном проекте соглашения именно такие.

_FR> и вот почему: если в тело ифа понадобится добавить ещё одно выражение, то мне не придётся подниматься вверх ставить открывающую кавыку, спускаться вниз и ставить кавыку (то есть совершать в три раза больше действий).

Ничего страшного. Не надо жечь газ всю ночь экономя на спичках.

_FR>Так как ты употребил "не стОит", а не "нужно и требуется", то счёл возможным пофлеймить :о) Давно хотелось ;о)

ОК, беру свои слова обратно. Будем считать что вместо них надо читать "нужно и требуется".