2009-06-08 3 views
7

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

Например:

MyClass myClass = new MyClass(@"C:\temp"); 

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

Должен ли я переместить эти данные из конструктора и иметь его в качестве отдельного метода, такие как:

MyClass myClass = new MyClass(); 
myClass.LoadFromDirectory(@"C:\temp"); 

ответ

22

Может быть, вы должны попробовать это так со статическим методом, который возвращает экземпляр объекта.

var myClass = MyClass.LoadFromDirectory(@"C:\temp"); 

Это сохранит код инициализации вне вашего конструктора, а также предоставит вам объявление «одной строки», которое вы ищете.


Переход на комментарий снизу от плаката, добавляя государству реализацию может выглядеть примерно так:

public class MyClass 
{ 

#region Constructors 

    public MyClass(string directory) 
    { 
     this.Directory = directory; 
    } 

#endregion 

#region Properties 

    public MyClassState State {get;private set;} 

    private string _directory; 

    public string Directory 
    { 
     get { return _directory;} 
     private set 
     { 
      _directory = value; 
      if (string.IsNullOrEmpty(value)) 
       this.State = MyClassState.Unknown; 
      else 
       this.State = MyClassState.Initialized; 
     } 
    } 

#endregion 



    public void LoadFromDirectory() 
    { 
     if (this.State != MyClassState.Initialized || this.State != MyClassState.Loaded) 
      throw new InvalidStateException(); 

     // Do loading 

     this.State = MyClassState.Loaded; 
    } 

} 

public class InvalidStateException : Exception {} 


public enum MyClassState 
{ 
    Unknown, 
    Initialized, 
    Loaded 
} 
+0

Хорошая идея, инициализация и использование класса часто бывают разными. Это прекрасно отделяет их. Для еще большего разделения вы можете переместить логику инициализации в класс фабрики или строителя. – Mendelt

1

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

Целью конструктора является создание объекта. Цель метода - выполнить действие. Поэтому мой голос за эту форму:

MyClass myClass = new MyClass(); 
myClass.LoadFromDirectory(@"C:\temp"); 
+0

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

5

От этого зависит. Вы должны оценить основную цель этого класса. Какую функцию он выполняет?

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

Как правило, фаза инициализации не должна быть слишком интенсивной. Альтернативный способ делать выше может быть:

// Instantiate the class and get ready to load data from files. 
MyClass myClass = new MyClass(@"C:\temp"); 

// Parse the file collection and load necessary data. 
myClass.PopulateData(); 
+0

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

+5

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

+0

@ Jon Skeet, потрясающее, не лучшее место, но ваш C# в книге глубины мне очень помог. Благодаря! - Моя забота о том, что этот класс может занять несколько минут, поскольку он обрабатывает данные и имеет двойное состояние - это то, чего я надеялся избежать. –

0

Я думаю, вы должны решить между этими двумя подходами выше ("первая инициализация, то выполнить «vs» empty init, выполнить с параметрами ») в зависимости от того, планируете ли вы повторно использовать один и тот же объект для выполнения той же операции на входе differnt.
Если класс используется только для выполнения задачи с фиксированным параметром, я бы пошел с инициализацией в конструкторе (делая его только для чтения), а затем выполнил задачу по другому методу.
Если вы хотите продолжать выполнять задачу по различным параметрам, я бы поместил ее в сам метод задачи.

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

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

0

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

Рассмотрим разделив класс на две части:

interface IMyDataSource 
{ 
    // ... 
} 

class FileDataSource: IMyDataSource 
{ 
    public FileDataSource(string path) 
    { 
     // ... 
    } 
} 

class MyClass 
{ 
    public MyClass(IMyDataSource source) 
    { 
     // ... 
    } 
} 

IMyDataSource myDS = new FileDataSource(@"C:\temp"); 
MyClass myClass = new MyClass(myDS); 

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

+1

Но теперь FileDataSource имеет возможность бросать исключения в свой конструктор, таким образом просто передавая ошибки отдельному объекту. –

+0

@ Том Андерсон, это идея. FileDataSource предназначен для ввода-вывода, поэтому ожидаются исключения I/O в конструкторе. MyClass не предназначен для ввода-вывода, поэтому исключений I/O в конструкторе избегают. – finnw

0

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

0

Мое личное предпочтение было бы использовать инициализаторы C# 3.0.

class MyClass { 
    public string directory; 
    public void Load() { 
     // Load files from the current directory 
     } 
    } 

MyClass myClass = new MyClass{ directory = @"C:\temp" }; 
myClass.Load(); 

Это имеет несколько преимуществ:

  • Объект экземпляр не будет иметь автоматический файл на сторону системы эффекта.
  • Все аргументы называются.
  • Все аргументы являются необязательными (но, конечно, может бросить исключение в Load(), если не определено)
  • можно инициализировать как многие свойства, как вы хотите в инстанцирования вызова без необходимости перегружать конструктор. Например, параметры для того, следует ли переписывать каталоги, или подстановочный знак для файла для поиска для.
  • У вас все еще может быть некоторая логика в настройщике для каталога некоторые вещи, но опять-таки побочные эффекты обычно не очень хорошие.
  • При выполнении операций с файлами в отдельного вызова процедуры, вы избежите вопроса не в состоянии ссылки вашего MyClass экземпляр в обработчика исключений.
+0

Хотя все еще не плохой подход, он имеет хотя бы состояние, заданное или не заданное поле. –

0

Я собираюсь повторить «разделить их» здесь. Если это поможет, попробуйте следующее:

  1. Спросите себя: «Что делает этот метод/свойство/поле сделать
  2. Заставьте это сделать; Не больше, не меньше.

Применяя, что здесь, вы получите это:

  1. Конструктор должен создать объект.
  2. Ваш метод должен загружать свои данные из файловой системы.

Это, мне кажется, будет намного более логичным, чем «Конструктор должен создать объект и загрузить свои данные из файловой системы.

+0

Согласовано. Если вы хотите заблокировать и вернуться только после заполнения объекта обратно, используйте фабрику со статическим методом. – richardtallent

1

Я согласен с Ари и другими - разделить их.

Конструктор должен действительно выполнять минимальный объем работы (просто инициализируйте объект, готовый к использованию, и оставьте его на нем). Используя отдельный метод для выполнения работ:

  • Уточнив, что функция работника может занять много времени.
  • Легко предоставить несколько конструкторов для инициализации объекта с различной информацией (например, вы могли бы передать свой собственный класс (а не строку), который может предоставить путь. Или вы можете передать дополнительный параметр, который задает подстановочное имя файла для сопоставления или флаг, указывающий, следует ли повторять поиск в подпапках).
  • Вы избегаете проблем с конструктором. В конструкторе объект не полностью сформирован, поэтому может быть опасно выполнять работу - например, вызов виртуальной функции внутри конструктора - очень плохая идея. Чем меньше кода вы вводите в конструктор, тем меньше вероятность того, что вы сделаете что-то «плохое» случайно.
  • Это более чистый стиль кодирования, позволяющий разделить различные поведения/функции на отдельные методы. Сохраняйте инициализацию и работу отделенными
  • Сплит-класс будет легче обслуживать и рефакторировать в будущем.
Смежные вопросы