Сколько методов должно быть в классе?

htbcds3dl1emxblkn29nn5cqr0w.png


Пожалуй это один из важнейших вопросов, с которым сталкиваются разработчики, использующие классы. У нас есть принципы SOLID, есть свое видение прекрасного и огромное количество разночтений. Я не надеюсь дать исчерпывающий ответ «сколько методов должно быть в вашем классе». Но поделюсь мыслями, почему методов в классах должно быть меньше, намного меньше. Вплоть до 1.

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

После знакомства с основами ООП новички ассоциируют классы и объекты с кошечками и собачками. То есть под первым впечатлением объекты воспринимаются как проекции реального мира или некие модели, содержащие набор необходимых методов для решения задачи. И это прекрасное мнение об ООП, точнее чем оно должно быть.

Но после парочки коммерческих проектов представление рушится. В энтерпрайзе правят сервисы и анемичные модели. Не знаю, плохо это или хорошо, но это реальность. Поэтому на практике часто получаются толстые классы, не имеющие аналогий с реальным миром, и не поддающиеся однозначному обобщению. Часто их называют сервисами, часто они образуют слои. И понимание объекта опускается до уровня — группа методов с похожим назначением. Или методы, работающие с похожими данными. Или ещё с чем-то «похожим».

Каждый раз, начиная писать сервис, я долго думаю над названием… Бессмысленное AnotherEntityService снова и снова. А что если не должно быть тут никакого существительного? Как название сервиса отражает собранную внутри солянку запросов? Кто вообще пользуется этими методами одним разом?

Меня долго не отпускает пример рефакторинга от дядюшки Боба [Чистый Код, стр. 177]:

class Sql {
    constructor(private table: string, private columns: Column[]) {}

    public create(): string;
    public insert(fields: object[]): string;
    public selectAll(): string;
    public select(criteria: Criteria): string;

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

Вот результат преобразования.

abstract class Sql {
    constructor(private table: string, private columns: Column[]) {}

    public abstract generate(): string;
}

class CreateSql extends Sql {
    public generate(): string;
}

class SelectSql extends Sql {
    public generate(): string;
}

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

Что не менее важно, когда придёт время добавления команды Update, вам не придётся изменять ни один из существующих классов!

В ЧК не раз упоминается, что классы должны быть компактными. Но этот пример радует своей конкретикой, и он просто потрясный!

Я задумался, поскольку от разбиения столько плюсов, почему в реальности преобладают раздутые классы? Потому что мы раздробим классы до уровня функций и станет сложнее понимать общую картину? (А ФПшники по этому поводу комплексуют?) Или потому что из наших толстяков-сервисов инкапсуляция посыплется наружу? Мне кажется, существует темная сторона, которая хранит тайну больших классов.

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

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

И это тёмная сторона — приватные поля, приватные методы и зависимости. Вот чем чаще всего руководствуются разработчики, объединяя методы в сервисе. Порой кажется, что если в классе присутствуют все необходимые члены, то это лучшее место для нового метода! И сервис превращается в хранителя зависимостей и методов, которые в этих зависимостях нуждаются.

С каких пор общие зависимости что-то должны объединять? Похожесть внутренней реализации не является причиной создания абстракции. Другими словами, добавляя новый метод в сервис, потому что он «очень похож на другие методы» по своей реализации, мы нарушаем инкапсуляцию, SRP, OCP и ISP. Но я постоянно с этим сталкиваюсь.

Таким образом, на практике сервисная архитектура не образует высокоуровневых абстракций, а лишь представляет собой аналогию процедурщины. Причем худшую аналогию, так как процедуры не имеют лишних сущностей в виде псевдо-обобщающих имен сервисов и не тянут себе неиспользуемые зависимости. (Где-то злорадствуют ФПшники).

Давайте рассмотрим пример из официальной документации Angular.

@Injectable({ providedIn: 'root' })
export class HeroService {
 private heroesUrl = 'api/heroes';
 private httpOptions = { headers: new HttpHeaders({ 'Content-Type': 'application/json' }) };
 
 constructor(private http: HttpClient, private messageService: MessageService) {}
 
 getHero(id: number): Observable {
   const url = `${this.heroesUrl}/${id}`;
   return this.http.get(url).pipe(
     tap((_) => this.log(`fetched hero id=${id}`)),
     catchError(this.handleError(`getHero id=${id}`)),
   );
 }
 
 addHero(hero: Hero): Observable {
   return this.http.post(this.heroesUrl, hero, this.httpOptions).pipe(
     tap((newHero: Hero) => this.log(`added hero w/ id=${newHero.id}`)),
     catchError(this.handleError('addHero')),
   );
 }
 
 // ...
 
 private handleError(operation = 'operation', result?: T) {
   return (error: any): Observable => {
     // TODO: send the error to remote logging infrastructure
     console.error(error); // log to console instead
     // TODO: better job of transforming error for user consumption
     this.log(`${operation} failed: ${error.message}`);
     return of(result as T);
   };
 }
}
HeroService полностью
@Injectable({ providedIn: 'root' })
export class HeroService {
 
 private heroesUrl = 'api/heroes';  // URL to web api
 
 httpOptions = {
   headers: new HttpHeaders({ 'Content-Type': 'application/json' })
 };
 
 constructor(
   private http: HttpClient,
   private messageService: MessageService) { }
 
 /** GET heroes from the server */
 getHeroes(): Observable {
   return this.http.get(this.heroesUrl)
     .pipe(
       tap(_ => this.log('fetched heroes')),
       catchError(this.handleError('getHeroes', []))
     );
 }
 
 /** GET hero by id. Will 404 if id not found */
 getHero(id: number): Observable {
   const url = `${this.heroesUrl}/${id}`;
   return this.http.get(url).pipe(
     tap(_ => this.log(`fetched hero id=${id}`)),
     catchError(this.handleError(`getHero id=${id}`))
   );
 }
 
 /* GET heroes whose name contains search term */
 searchHeroes(term: string): Observable {
   if (!term.trim()) {
     // if not search term, return empty hero array.
     return of([]);
   }
   return this.http.get(`${this.heroesUrl}/?name=${term}`).pipe(
     tap(x => x.length ?
        this.log(`found heroes matching "${term}"`) :
        this.log(`no heroes matching "${term}"`)),
     catchError(this.handleError('searchHeroes', []))
   );
 }
 
 //////// Save methods //////////
 
 /** POST: add a new hero to the server */
 addHero(hero: Hero): Observable {
   return this.http.post(this.heroesUrl, hero, this.httpOptions).pipe(
     tap((newHero: Hero) => this.log(`added hero w/ id=${newHero.id}`)),
     catchError(this.handleError('addHero'))
   );
 }
 
 /** DELETE: delete the hero from the server */
 deleteHero(hero: Hero | number): Observable {
   const id = typeof hero === 'number' ? hero : hero.id;
   const url = `${this.heroesUrl}/${id}`;
 
   return this.http.delete(url, this.httpOptions).pipe(
     tap(_ => this.log(`deleted hero id=${id}`)),
     catchError(this.handleError('deleteHero'))
   );
 }
 
 /** PUT: update the hero on the server */
 updateHero(hero: Hero): Observable {
   return this.http.put(this.heroesUrl, hero, this.httpOptions).pipe(
     tap(_ => this.log(`updated hero id=${hero.id}`)),
     catchError(this.handleError('updateHero'))
   );
 }
 
 /**
  * Handle Http operation that failed.
  * Let the app continue.
  * @param operation - name of the operation that failed
  * @param result - optional value to return as the observable result
  */
 private handleError(operation = 'operation', result?: T) {
   return (error: any): Observable => {
 
     // TODO: send the error to remote logging infrastructure
     console.error(error); // log to console instead
 
     // TODO: better job of transforming error for user consumption
     this.log(`${operation} failed: ${error.message}`);
 
     // Let the app keep running by returning an empty result.
     return of(result as T);
   };
 }
 
 /** Log a HeroService message with the MessageService */
 private log(message: string) {
   this.messageService.add(`HeroService: ${message}`);
 }
}

Перед нами типичный сервис. (Я бы даже сказал «бест практикс»). И на первый взгляд всё с ним нормально. HeroService работает с Hero сущностями, ничего лишнего. Много лишнего! Ни один клиент не использует интерфейс согласовано — только частями.

Что делать, когда появится новый АПИ метод для Hero? Когда нужно остановиться складывать всё в одно, и разбить класс? Уверен, что каждый задавался этими вопросами. Но само наличие этого сервиса в проекте только смущает разработчиков, подталкивая их к написанию кода через изменение, а не расширение.

Давайте разберёмся, что делает «похожими» методы, собранные в HeroService, и как это разбивается.

  • Все методы пользуются общим url — решается вынесением в константу BASE_HERO_API_URL.
  • Часть методов пользуется одинаковыми http-заголовками — решается вынесением в интерсептор. Или можно выделить специальный HttpClient, который обернёт методы get, post, put, delete необходимыми опциями.
  • handleError — решается вынесением в отдельный класс, который отлично выступит на роли ответственного по обработке ошибок.

Получилось, что все объединяющие факторы — это необязательные детали реализации, которые могли бы и отличаться, не будь на то воли случайных обстоятельств. Внешних причин для объединения не было. Мы пишем веб, и это странно, когда одна кнопка сперва вызывает addHero, а потом пользуется другой функциональностью HeroService.

Покажи мне код!
@Injectable({ providedIn: 'root' })
export class GetHeroesOperation {
 constructor(
   private http: HeroHttpClient,
   private messageService: MessageService,
   private errorHandler: ErrorHandler,
 ) {}
 
 execute(): Observable {
   return this.http.get().pipe(
     tap((_) => this.messageService.add('fetched heroes')),
     catchError(this.errorHandler.handleError('getHeroes', [])),
   );
 }
}
 
@Injectable({ providedIn: 'root' })
export class AddHeroOperation {
 constructor(
   private http: HeroHttpClient,
   private messageService: MessageService,
   private errorHandler: ErrorHandler,
 ) {}
 
 execute(hero: Hero): Observable {
   return this.http.post('', hero).pipe(
     tap((newHero: Hero) => this.logAddedHero(newHero)),
     catchError(this.errorHandler.handleError('addHero')),
   );
 }
 
 private logAddedHero(newHero: Hero): void {
   return this.messageService.add(`AddHeroOperation: added hero w/ id=${newHero.id}`);
 }
}
 
@Injectable({ providedIn: 'root' })
export class ErrorHandler {
 constructor(private messageService: MessageService) {}
 
 handleError(operation: string, result?: T) {
   return (error: any): Observable => {
     // TODO: send the error to remote logging infrastructure
     console.error(error); // log to console instead
 
     // TODO: better job of transforming error for user consumption
     this.messageService.add(`${operation} failed: ${error.message}`);
 
     // Let the app keep running by returning an empty result.
     return of(result as T);
   };
 }
}
 
@Injectable({ providedIn: 'root' })
export class HeroHttpClient {
 private httpOptions = { headers: new HttpHeaders({ 'Content-Type': 'application/json' }) };
 
 constructor(@Inject(BASE_HERO_API_URL) private baseUrl: string, private http: HttpClient) {}
 
 get(endpoint = ''): Observable {
   return this.http.get(this.getUrl(endpoint));
 }
 
 private getUrl(endpoint: string): string {
   return Location.joinWithSlash(this.baseUrl, endpoint);
 }
 
 post(endpoint = '', data: unknown): Observable {
   return this.http.post(this.getUrl(endpoint), data, this.httpOptions);
 }
 
 put(endpoint = '', data: unknown): Observable {
   return this.http.put(this.getUrl(endpoint), data, this.httpOptions);
 }
 
 delete(endpoint = ''): Observable {
   return this.http.delete(this.getUrl(endpoint), this.httpOptions);
 }
}
 

Я не пытаюсь (и не хочу) делать заявлений, типа: «У класса не может быть больше 1-го метода!». Конечно может, когда у объекта есть состояние, в силу вступают другие правила. Так же я не против включения в класс методов, которые должны использоваться согласованно: isExecutable, validateParams, execute.

То что я говорю, касается в большей степени сервисов-монстров. Так как сервисы чаще всего не представляют собой вменяемой абстракции. И лучше бы акцент смещался в сторону их уменьшения. Для себя я сформулировал следующий принцип:

Класс не должен содержать в себе набор методов только по причине их схожей реализации.

Следуя этому правилу, можно обнаружить потерянные элементы системы, которые скрываются в приватных методах. Такой подход поможет соблюдать OCP и ISP в буквальном смысле. Когда ваши фичи реально расширяют программу, а не добавляют методы в 10 классов в 10 слоях.

Надеюсь, вам понравилось. Спасибо! Пока!

© Habrahabr.ru