Нарушает ли класс CommaDelimLog в следующем коде принцип единой ответственности?

Программа анализирует лог-файлы - каждый лог-файл может иметь разный формат полей (фиксированная ширина, разделители-запятые и т. д.). Кроме того, каждый файл журнала состоит из нескольких разных типов журналов - каждый вид имеет различное определение поля). Например, файл журнала CSV может выглядеть так:

Файл журнала А

logType1, 10/1/2012, 12, abc
logType2, a, b, c, d, 11/1/2012
logType1, 10/2/2012, 21, def
logType2, e, f, c, d, 12/1/2012
logType3, 3.23, ....

Ниже приведен код. Сколько твердых принципов было нарушено в следующем коде? Один парень сказал, что список определения макета не следует смешивать с журналом синтаксического анализа. Так это как минимум нарушает SRP (или больше)? Каков наилучший способ рефакторинга структуры?

// Field
public interface IField { .... }
public class Field : IField { ... common field methods, etc.... }
public class FixedWidthField : Field { }
public class CommaDelimField : Field { ... }

// Log type
public interface ILogType<out T> where T : IField { ... IEnumerable<T> Fields { get; } }
public class LogType<T> : ILogType<T> where T : IField 
{ .... 
    public LogType(..., List<T> fields) { ... Fields = fields; }
}

// File
public inteface ILogFile<out T> where T: IField { ... IEnumerable<ILogType<T>> LogTypeList { get; set; } }
public abstract class LogFile<T> : ILogFile<T> where T: IField 
{ ....
    public IEnumerable<ILogType<T>> LogTypeList { get; set; }
    public virtual string Row { get { ... } set { ...} }
    public string GetParsedFieldString() { ... }
}
public class CommaDelimLog : LogFile<CommaDelimField>
{
     public override string Row { get { ... } set { ...code to parse the line...} }
     public override string GetParsedFieldString() { ... }
}

// The following initilize code store all the layout information
public static List<ILogFile<IField>> LogFileList = new List<ILogFile<IField>>
{
    new CommaDelimLog("logFileA", ...., new List<ILogType<CommaDelimField>> {
        new LogType<CommaDelimField>("logType1", ... new List<CommaDelimField>{ .... }
        new LogType<CommaDelimField>("logType2", ... new List<CommaDelimField>{ .... }
        ....
    }),
    new CommaDelimLog("logFileB", .... a long long list

Основная программа получает элемент из LogFileList в соответствии с шаблоном имени файла, читает файлы журнала построчно и присваивает свойство Row, а затем получает проанализированную строку.


.net c# solid-principles single-responsibility-principle open-closed-principle
person ca9163d9    schedule 12.10.2012    source источник

Ответы (1)


arrow_upward
1
arrow_downward

Это, вероятно, нарушает принцип открытого-закрытого из-за наследования поведения от LogFile (хотя GetParsedFieldString на самом деле не является виртуальным в абстрактной базе, и трудно быть уверенным без дополнительного контекста).

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

Кажется, вы в основном спрашиваете о принципе открытия-закрытия на основе тега вопроса, но принцип инверсии зависимостей также нарушается, поскольку ваши конкретные классы напрямую зависят от других конкретных классов, например CommaDelimLog, полагаясь на CommaDelimField.

Например, с ninject вы можете сделать что-то вроде:

Bind<IField>().To<CommaDelimField>().WhenInjectedInto<CommaDelimLog>();

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

Могут быть и другие нарушения, но я уступлю другим.

person David    schedule 12.10.2012
comment
Можете ли вы уточнить поведение из-за наследования от LogFile? - person ca9163d9; 13.10.2012
comment
GetParsedFieldString в LogFile — это место, где находится логика класса, но оно не является абстрактным, поэтому я предполагаю, что существует некоторая базовая реализация, которая будет совместно использоваться некоторыми наследниками. Это может привести к тому, что в будущем потребуется изменить поведение этого метода в базовом классе. Принцип открытости/закрытости заключается в проектировании ваших классов так, чтобы они были открыты для расширения, но закрыты для модификации, поэтому правила, которые потенциально могут измениться, не должны реализовываться в базовых классах в соответствии с принципом. - person David; 15.10.2012