Методы, которые обертывают один метод - PullRequest
6 голосов
/ 24 февраля 2010

Я думаю, что схожу с ума, кто-то, пожалуйста, успокой меня.

public class MyFile
{   
    public static byte[] ReadBinaryFile(string fileName)
    {
        return File.ReadAllBytes(fileName);
    }

    public static void WriteBinaryFile(string fileName, byte[] fileContents)
    {
        File.WriteAllBytes(fileName, fileContents);
    }
}

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

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

Ответы [ 6 ]

10 голосов
/ 24 февраля 2010

Как написано, класс / методы являются мусором. Тем не менее, я вижу ситуацию, когда аналогичный шаблон может быть использован законно:

public interface IFileStorage
{
    byte[] ReadBinaryFile(string fileName);
    void WriteBinaryFile(string fileName, byte[] fileContents);
}

public class LocalFileStorage : IFileStorage { ... }

public class IsolatedFileStorage : IFileStorage { ... }

public class DatabaseFileStorage : IFileStorage { ... }

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

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

6 голосов
/ 24 февраля 2010

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

Взять, к примеру, если у вас был такой код

File.WriteAllBytes(fileName, fileContents);

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

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

4 голосов
/ 24 февраля 2010

Существование этих методов - отвратительная аберрация, которая должна быть немедленно исправлена.

Вероятно, они были написаны кем-то, кто не знал о классе File, а затем были переписаны кем-то, кто знал, но не был таким смелым, как вы.

2 голосов
/ 24 февраля 2010

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

  1. Чтобы отделить код от статического класса . Если MyFile не является статическим, то это может служить абстракцией над статическим IO.File объектом. Код, который напрямую обращается к файловой системе, может быть очень сложным для тестирования, поэтому подобные абстракции могут быть полезны. (Например, у меня есть интерфейс IFileSystem, от которого зависит весь мой код ввода-вывода, и мой класс FileSystem реализует этот интерфейс и упаковывает основные методы File.)

  2. Для сохранения согласованных уровней абстракции в методе . Мне не нравится смешивать код в проблемной области (например, «клиенты» и «заказы») с кодом в области _solution) (файлы, байты и биты). Я часто пишу такие обертки, чтобы облегчить чтение кода и дать мне очки расширяемости. Я бы лучше прочитал GetDataFileContents(), чем File.ReadAllBytes("foo.dat").

  3. Для предоставления контекста . Если фрагмент кода выполняется для его побочных эффектов, таких как удаление текстового файла для выполнения удаления заказа клиента, тогда я предпочел бы читать DeleteCustomerOrderFile("foo.txt"), чем File.Delete("foo.txt"). Первый предоставляет контекстную информацию для кода, последний - нет.

2 голосов
/ 24 февраля 2010

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

Добавлены ли эти методы-заполнители для последующей логики? Если это так, то следует добавить комментарии или даже страшный оператор TODO, чтобы кто-то позже обернулся и завершил мысль.

0 голосов
/ 24 февраля 2010

Полагаю, что ответ на этот вопрос зависит от вашей команды, практики и того, для чего предназначена цель ultiamte (например, фрагмент кода, который вы в данный момент записывает в файл, но будет записывать в веб-службу / базу данных / morse-code machine, как только он закончил - хотя это "оправдание" вроде бы побеждено именами классов / методов). Я думаю, что вы сами ответили на вопрос: «В нашей команде мы нацелены на ЯГНИ, и это, похоже, бросает вызов этому».

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

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