FxCop — ошибка CA1034 — ПОЧЕМУ?

Я выполняю статический анализ кода с помощью FxCop 1.36 и продолжаю получать предупреждение CA1034: NestedTypesShouldNotBeVisible.

Я бы понял, если бы родительский класс был объявлен как внутренний или частный, но он общедоступный. Почему было бы плохо, если бы TimerReset был объявлен общедоступным?

Я что-то пропустил, или это то, что можно игнорировать?

Спасибо за любой вклад!

Вот фрагмент кода, вызывающего это предупреждение:

namespace Company.App.Thing
{
    public partial class Page : XtraPage
    {
        public delegate void TimerResetDelegate(object sender, EventArgs e);
        private TimerResetDelegate _timerReset;

        public Page()
        {
            InitializeComponent();
        }

        public TimerResetDelegate TimerReset
        {
            set
            {
                if (null != (_timerReset = value))
                {
                    checkBox.Click += new EventHandler(_timerReset);
                    textField.Click += new EventHandler(_timerReset);
                    textField.KeyDown += new KeyEventHandler(_timerReset);
                    TimeField.Click += new EventHandler(_timerReset);
                    TimeField.KeyDown += new KeyEventHandler(_timerReset);
                }
            }
        }
    }
}

static-analysis c# fxcop
person Tim    schedule 22.07.2009    source источник
comment
Почему вы не используете EventHandler?   -  person Sam Harwell    schedule 23.07.2009
comment
Это был и мой вопрос.   -  person Ed S.    schedule 23.07.2009
comment
Просто к сведению любопытных читателей: вместо того, чтобы определять set или get/set для такого «чисто технического» свойства TimerReset, лучше написать его как event с add/remove. см. MSDN   -  person quetzalcoatl    schedule 22.06.2020

Ответы (5)


arrow_upward
5
arrow_downward

Вообще говоря, вложенные типы сложнее «обнаружить».

Например. Чтобы использовать ваш вложенный тип, мне придется написать следующее

Page.TimerResetDelegate timer = new Page.TimerResetDelegate();

Несмотря на то, что выше приведен допустимый код C#, он не читается как обычное использование типа.

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

person SolutionYogi    schedule 22.07.2009

arrow_upward
3
arrow_downward

Почему было бы плохо, если бы TimerReset был объявлен общедоступным?

В точности так, как указано в описании:

Вложенные типы полезны для инкапсуляции частных деталей реализации содержащего типа. Используемые для этой цели вложенные типы не должны быть видны извне.

Поскольку вы публично раскрываете TimerResetDelegate с помощью TimerReset, я думаю, это не детали реализации.

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

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

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

Поскольку TimerResetDelegate является делегатом, это на самом деле не применимо.

Переместите TimerResetDelegate в собственный файл TimeResetDelegate.cs и поместите его в свое пространство имен Company.App.Thing. Затем он больше не является вложенным.

Конечно, было бы еще лучше использовать EventHandler вместо определения собственного типа делегата.

person Mark Brackett    schedule 22.07.2009

arrow_upward
2
arrow_downward

Это потому, что ваш делегат является типом, но он определен в классе Page. Я бы просто определил его в пространстве имен Company.App.Thing, но на самом деле это не проблема. Если бы вы писали API, это просто сделало бы его немного запутанным, вот и все.

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

person Ed S.    schedule 22.07.2009
comment
Это не возвращение делегата. Это свойство только для установки, оно предназначено для установки/регистрации обработчика. Page.TimerReset = new handler(...). Я полагаю, что ОП не знал, что в событии можно написать add/remove, и поэтому он решил использовать установщик свойств. - person quetzalcoatl; 22.06.2020

arrow_upward
2
arrow_downward

ИМХО, это правило FxCop, которое можно игнорировать.

На уровне CLR нет ничего плохого в наличии вложенного класса. Это просто руководящее правило, добавленное в FxCop, потому что авторы считают его менее удобным или менее удачным, чем создание класса без вложенности.

person JaredPar    schedule 22.07.2009
comment
FxCop предупреждает о том, что вложенный тип является общедоступным, а не об использовании вложенного типа. - person SolutionYogi; 23.07.2009

arrow_upward
-2
arrow_downward

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

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

person Thorarin    schedule 22.07.2009
comment
Это не отвечает на вопрос. - person srm; 11.09.2017
comment
Это не отвечает на вопрос. По сути, он просто суммирует проблему. - person quetzalcoatl; 22.06.2020