Передача переменной-члена в качестве параметра метода

В проекте я нашел такой код:

class SomeClass
{
    private SomeType _someField;

    public SomeType SomeField
    {
        get { return _someField; }
        set { _someField = value; }
    }

    protected virtual void SomeMethod(/*...., */SomeType someVar)
    {
    }

    private void SomeAnotherMethod()
    {
        //.............
        SomeMethod(_someField);
        //.............
    }

};

Как убедить моих товарищей по команде, что это плохой код?

Я считаю, что это ненужное осложнение. Зачем передавать переменную-член как параметр метода, если у вас уже есть доступ к ней? Это также является нарушением инкапсуляции.

Вы видите какие-либо другие проблемы с этим кодом?

31 голос | спросил tika 29 PMpSun, 29 Apr 2012 19:39:15 +040039Sunday 2012, 19:39:15

10 ответов


2

Я думаю, что это правильная тема, но причина, по которой вы получаете смешанные ответы, связана с тем, как был сформирован вопрос. Лично у меня был тот же опыт с моей командой, когда передача членов в качестве аргументов была излишней и запутанной. У нас будет класс, который работает с набором элементов, но некоторые функции напрямую получат доступ к элементам, а другие функции будут изменять одни и те же элементы с помощью параметров (т. Е. Использовать совершенно разные имена), и для этого не было никакой технической причины. По техническим причинам я имею в виду пример, который предоставила Кейт.

Я бы рекомендовал сделать шаг назад и вместо того, чтобы точно сосредоточиться на передаче членов в качестве параметров, инициировать обсуждения с вашей командой на ясности и удобочитаемости. Либо более формально, либо просто в коридорах обсудите, что облегчает чтение некоторых сегментов кода и других сегментов кода. Затем определите критерии качества или атрибуты чистого кода, которые как команда, к которой вы хотели бы стремиться. В конце концов, даже когда мы работаем над проектами «зеленого поля», мы тратим более 90% времени на чтение, и как только код написан (скажем, через 10-15 минут), он переходит в обслуживание, где читаемость еще важнее.

Итак, для вашего конкретного примера здесь аргумент, который я хотел бы использовать, заключается в том, что меньше кода всегда легче читать, чем больше кода. Функция, имеющая 3 параметра, сложнее обрабатывать мозг, чем функция, у которой нет ни одного параметра, ни одного параметра. Если есть другое имя переменной, мозг должен отслеживать еще одну вещь при чтении кода. Так что помните «int m_value», а затем «int localValue» и помните, что на самом деле это означает, что другой всегда дороже для вашего мозга, а затем просто работает с «m_value».

Для большего количества боеприпасов и идей я бы рекомендовал собрать копию дяди Боба Clean Code .

ответил DXM 29 PMpSun, 29 Apr 2012 22:13:53 +040013Sunday 2012, 22:13:53
28

Я могу представить обоснование для передачи поля члена в качестве параметра в (private) методе: он делает явным, на что зависит ваш метод.

Как вы говорите, все поля-члены являются неявными параметрами вашего метода, потому что весь объект. Однако нужен ли полный объект для вычисления результата? Если SomeMethod - внутренний метод, который зависит только от _someField, не является ли это более ясным, чтобы сделать эту зависимость явной? Фактически, если сделать эту зависимость явной, можно также намекнуть, что вы можете фактически реорганизовать этот кусок кода из своего класса! (Заметьте, я предполагаю, что мы не говорим о геттерах или сеттерах здесь, но код, который на самом деле что-то вычисляет)

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

ответил Andres F. 29 PMpSun, 29 Apr 2012 20:17:53 +040017Sunday 2012, 20:17:53
24

Я вижу одну серьезную причину передачи переменных-членов в качестве аргументов функции private - чистота функции. Переменные-члены являются фактически глобальным состоянием с точки зрения, более того, изменяемым глобальным состоянием, если указанный член изменяется во время выполнения метода. Заменяя ссылки на переменные-члены с помощью параметров метода, мы можем эффективно сделать функцию чистой. Чистые функции не зависят от внешнего состояния и не имеют побочных эффектов, всегда возвращая одни и те же результаты, учитывая тот же набор входных параметров, что облегчает тестирование и дальнейшее обслуживание.

Конечно, нелегко и практично, чтобы все ваши методы были чистыми методами на языке ООП. Но я считаю, что вы многое понимаете с точки зрения ясности кода, используя методы, которые обрабатывают сложную логику и используют неизменяемые переменные, сохраняя при этом нечистое «глобальное» управление состоянием в рамках выделенных методов.

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

ответил scrwtp 30 AMpMon, 30 Apr 2012 02:27:01 +040027Monday 2012, 02:27:01
7

Если функция вызывается разным образом, иногда передавая эту переменную-член, а иногда передавая что-то еще, тогда все в порядке. Например, я бы вообще не подумал об этом:

if ( CalculateCharges(newStartDate) > CalculateCharges(m_StartDate) )
{
     //handle increase in charges
}

где newStartDate - это локальная переменная и m_StartDate является переменной-членом.

Однако, если функция только когда-либо вызывается с переданной ей переменной-членом, это нечетно. Функции-члены постоянно работают с переменными-членами. Они могут делать это (в зависимости от языка, в котором вы работаете), чтобы получить копию переменной-члена - если это так, и вы не видите этого, код может быть приятнее, если бы весь процесс был явным.

ответил Kate Gregory 29 PMpSun, 29 Apr 2012 21:17:41 +040017Sunday 2012, 21:17:41
1

В графических интерфейсах GUI обычно есть своего рода класс «Вид», который представляет объекты, изображенные на экране, и этот класс обычно предоставляет такой метод, как invalidateRect(Rect r), чтобы отметить часть своей области рисования, которую нужно перерисовать. Клиенты могут вызвать этот метод для запроса обновления части представления. Но представление может также вызвать свой собственный метод, например:

invalidateRect(m_frame);

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

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

invalidateFrame();

Но зачем добавлять специальный метод только для этого, когда вы можете использовать более общий invalidateRect()? Или, если вы решили предоставить invalidateFrame(), вы, скорее всего, реализуете его с точки зрения более общего invalidateRect():

View::invalidateFrame(void)
{
    invalidateRect(m_frame)
}
  

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

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

ответил Caleb 29 PMpSun, 29 Apr 2012 22:25:20 +040025Sunday 2012, 22:25:20
1

Что никто не затронул, так это то, что SomeMethod защищен виртуальным. Это означает, что производный класс может использовать его и повторно реализовать его функциональность. У производного класса не будет доступа к частной переменной и, следовательно, он не сможет обеспечить пользовательскую реализацию SomeMethod, которая зависит от частной переменной. Вместо того, чтобы принимать зависимость от частной переменной, декларация требует, чтобы вызывающий ее передал.

ответил Michael Brown 30 AMpMon, 30 Apr 2012 04:24:10 +040024Monday 2012, 04:24:10
1

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

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

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

ответил James Anderson 30 AMpMon, 30 Apr 2012 11:59:31 +040059Monday 2012, 11:59:31
1

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

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

Учитывая все это, я не стал бы говорить о том, что код «плохо» как таковой, если вы передаете переменную-член в один из методов этого класса. Однако я бы предположил, что он, как правило, не идеален, поскольку он может стимулировать много дублирования кода, где параметр присваивается локальной переменной, например, а дополнительные параметры добавляют «шум» в код, где он не нужен. Если вы являетесь поклонником книги «Чистый код», вы знаете, что в ней упоминается, что вы должны сохранить количество параметров метода до минимального минимума и только если нет другого более разумного способа для метода получить доступ к параметру .

ответил S.Robins 3 Mayam12 2012, 08:05:44
1

Некоторые вещи, которые приходят ко мне, когда мы об этом думаем:

  1. В общем, сигнатуры методов с меньшим количеством параметров легче понять; были изобретены методы большой причины, заключающиеся в том, чтобы исключить длинные списки параметров, выйдя замуж за их данные, на которых они работают.
  2. Создание подписи метода зависит от переменной-члена, что затруднит изменение этих переменных в будущем, потому что вам нужно не только изменить внутри метода, но и везде, где также вызывается метод. И поскольку SomeMethod в вашем примере защищен, подклассы также должны будут измениться.
  3. Методы (публичные или частные), которые не зависят от внутренних элементов класса, не обязательно должны быть в этом классе. Их можно было бы разделить на полезные методы и быть такими же счастливыми. У них нет ни одного бизнеса, входящего в этот класс. Если ни один другой метод не зависит от этой переменной после перемещения метода (ов), то эта переменная тоже должна идти! Скорее всего, переменная должна быть на своем собственном объекте с помощью этого метода или методов, которые действуют на него, становясь общедоступными и составляются родительским классом.
  4. Передача данных по различным функциям, таким как ваш класс, - это процедурная программа с глобальными переменными, которая просто летит перед дизайном OO. Это не намерение переменных-членов и функций-членов (см. Выше), и это звучит так, как будто ваш класс не очень сплочен. Я думаю, что это запах кода, который предлагает лучший способ группировать ваши данные и методы.
ответил Colin Hunt 20 AM00000090000003531 2013, 09:15:35
0

Вам не хватает, если параметр («аргумент») является ссылкой или только для чтения.

class SomeClass
{
    protected SomeType _someField;
    public SomeType SomeField
    {
        get { return _someField; }

        set {
          if (doSomeValidation(value))
          {
            _someField = value;
          }
        }
    }

    protected virtual void ModifyMethod(/*...., */ ref SomeType someVar)
    { 
      // ...
    }    

    protected virtual void ReadMethod(/*...., */ SomeType someVar)
    { 
      // ...
    }

    private void SomeAnotherMethod()
    {
        //.............

        // not recommended, but, may be required in some cases
        ModifyMethod(ref this._someField);

        //.............

        // recommended, but, verbose
        SomeType SomeVar = this.someField;
        ModifyMethod(ref SomeVar);
        this.someField = SomeVar;

        //.............

        ReadMethod(this.someField);
        //.............
    }

};

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

Помните, что у «сеттеров» могут быть дополнительные методы, а не просто назначать, а иногда даже быть виртуальными методами или вызывать виртуальные методы.

Примечание. Я предлагаю сохранить внутренние поля свойств как «защищенные».

ответил umlcat 30 AMpMon, 30 Apr 2012 01:31:05 +040031Monday 2012, 01:31:05

Похожие вопросы

Популярные теги

security × 330linux × 316macos × 2827 × 268performance × 244command-line × 241sql-server × 235joomla-3.x × 222java × 189c++ × 186windows × 180cisco × 168bash × 158c# × 142gmail × 139arduino-uno × 139javascript × 134ssh × 133seo × 132mysql × 132