Ваш подход к добавлению вашего нового поведения только спас вас от инициализации в общей схеме вещей, потому что вы в любом случае реализовали CloudAclManager отдельно от CloudFileManager. Я не согласен с некоторыми вещами о том, как это интегрируется с вашим существующим дизайном (что неплохо) ...
Что с этим не так?
- Вы разделили свои файловые менеджеры и использовали
IFileManager, но вы не сделали то же самое с IAclManager. Хотя у вас есть фабрика для создания различных файловых менеджеров, вы автоматически сделали CloudAclManager IAclManager из CloudFileManager. Итак, какой смысл иметь IAclManager?
- Что еще хуже, ты
инициализировать новый
CloudAclManager
внутри CloudFileManager каждый раз, когда вы пытаетесь получить его ACL
менеджер - вы только что дали фабрику
обязанности к вашему
CloudFileManager.
- У вас есть
CloudFileManager орудия IAclManager поверх его как свойства. Вы просто перенесли правило, согласно которому разрешения CloudFileManager уникальны, на уровень модели, а не на уровень бизнес-правил. Это также приводит к поддержке ненужного
потенциал круговой ссылки между собой и собственностью.
- Даже если бы вы хотели
CloudFileManager делегировать
функциональность разрешения для
CloudAclManager, зачем вводить в заблуждение других
классы думать, что
CloudFileManager обрабатывает свои собственные
наборы разрешений? Вы только что сделали свой
Модель класса выглядит как фасад.
Хорошо, так что мне делать вместо этого?
Во-первых, вы назвали свой класс CloudFileManager, и это правильно, потому что его единственная ответственность - управлять файлами для облака. Теперь, когда наборы разрешений также должны управляться для облака, действительно ли правильно для CloudFileManager взять на себя эти новые обязанности? Ответ - нет.
Это не означает, что у вас не может быть кода для управления файлами и кода для управления разрешениями в одном классе. Однако тогда было бы более разумно, чтобы класс назывался чем-то более общим, например CloudFileSystemManager, поскольку его обязанности не будут ограничиваться только файлами или разрешениями.
К сожалению, если вы переименуете свой класс, это отрицательно скажется на тех, кто в данный момент использует ваш класс. Так как насчет использования композиции, но без изменения CloudFileManager?
Мое предложение было бы сделать следующее:
1. Сохраните IAclManager и создайте IFileSystemManager
public interface IFileSystemManager {
public IAclManager AclManager { get; }
public IFileManager FileManager { get; }
}
или
public interface IFileSystemManager : IAclManager, IFileManager {
}
2. Создать CloudFileSystemManager
public class CloudFileSystemManager : IFileSystemManager {
// implement IFileSystemManager
//
// How each manager is set is up to you (i.e IoC, DI, simple setters,
// constructor parameter, etc.).
//
// Either way you can just delegate to the actual IAclManager/IFileManager
// implementations.
}
Почему?
Это позволит вам использовать ваше новое поведение с минимальным воздействием на текущую кодовую базу / функциональность, не затрагивая тех, кто использует ваш оригинальный код. Управление файлами и управление разрешениями также могут совпадать (то есть проверять разрешения перед попыткой действительного действия с файлом). Это также расширяемо, если вам нужен какой-либо другой менеджер набора разрешений или любой другой тип менеджеров по этому вопросу.
РЕДАКТИРОВАТЬ - Включая уточняющие вопросы Аскера
Если я создам IFileSystemManager : IFileManager, IAclManager, будет ли хранилище по-прежнему использовать FileManagerFactory и возвращать экземпляр CloudFileSystemManager?
Нет, FileManagerFactory не должно возвращать FileSystemManager. Ваша оболочка должна будет обновиться, чтобы использовать новые интерфейсы / классы. Возможно что-то вроде следующего:
private IAclManagerFactory m_aclMgrFactory;
private IFileManagerFactory m_fileMgrFactory;
public UserFileRepository(IAclManagerFactory aclMgrFactory, IFileManagerFactory fileMgrFactory) {
this.m_aclMgrFactory = aclMgrFactory;
this.m_fileMgrFactory = fileMgrFactory;
}
public void CreateResume() {
// I understand that the determination of "cloudManager"
// is non-trivial, but that part doesn't change. For
// your example, say environment = "cloudManager"
var environment = GetEnvMgr( ... );
var fileManager = m_fileMgrFactory.GetManager(environment);
fileManager.AddFile();
// do permission stuff - see below
}
Что касается вызова разрешений, у вас есть несколько вариантов:
// can use another way of determining that a "cloud" environment
// requires permission stuff to be done
if(environment == "cloudManager") {
var permissionManager = m_aclMgrFactory.GetManager(environment);
permissionManager.AddPermission();
}
или
// assumes that if no factory exists for the environment that
// no permission stuff needs to be done
var permissionManager = m_aclMgrFactory.GetManager(environment);
if (permissionManager != null) {
permissionManager.AddPermission();
}