3
\$\begingroup\$

I have a component with this template:

<ion-card *ngIf="isActive">
  <ion-card-header>{{question.question}}</ion-card-header>

  <ion-grid>
    <ion-row>
      <ion-col *ngFor="let answer of question.answers">
        <button ion-button outline (click)="answer.action()">{{answer.text}}</button>
      </ion-col>
    </ion-row>
  </ion-grid>

</ion-card>

And the code:

@Component({
  selector: 'extra-card',
  templateUrl: 'extra-card.html'
})
export class ExtraCard {
  public static readonly EXTRA_CARD_TYPE_1: string = "type_1";
  public static readonly EXTRA_CARD_TYPE_2: string = "type_2";

  @Input() type: string = ExtraCard.EXTRA_CARD_TYPE_1;

  isActive: boolean = true;
  question;

  constructor(
    private ga: GoogleAnalytics,
    private socialSharing: SocialSharing,
    private appRateService: AppRateService
  ) {}

  ngOnInit() {
    switch (this.type) {
      case ExtraCard.EXTRA_CARD_TYPE_1:
        this.makeExtraCardType1();
        break;

      case ExtraCard.EXTRA_CARD_TYPE_2:
        this.makeExtraCardType2();
        break;
    }
  }

  private makeExtraCardType1() {
    this.question = {
      question: "Some question",
      answers: [
        {text: "Answer 1", action: () => {this.action1Type1();}},
        {text: "Answer 2", action: () => {this.action2Type1();}},
        {text: "Answer 3", action: () => {this.action3Type1();}}
      ]
    };
  }

  private makeExtraCardType2() {
    this.question = {
      question: "Some question",
      answers: [
        {text: "Answer 1", action: () => {this.action1Type2();}},
        {text: "Answer 2", action: () => {this.action2Type2();}},
      ]
    };
  }

...

}

What does this code do?

A sequence of questions is shown to the user. Each question has several options. In the end of the sequence we do some action and skip this card.

What is the problem?

In this example I have 2 types of extra cards, but I want to have more (5, 10, 20, etc). In this case my component code will be growing too quickly.

What I want

I want to separate the logic of different question sequences. But I'm facing a problem with dependency injection for some unique actions in sequences. And I want to have a single component and use it like this:

<extra-card [type]="type_1"></extra-card>

Also, I want to avoid excessive dependency injections in the base component (in case with passing DI instances to question model).

\$\endgroup\$
  • \$\begingroup\$ stackoverflow.com/a/126455/482868 describes how switch statements can be eliminated if Strategy pattern is applied. Did you look into it? I can write a sketch of a solution in a few hours. \$\endgroup\$ – Igor Soloydenko Sep 22 '17 at 16:55
  • \$\begingroup\$ @IgorSoloydenko no I did not. I can imagine how to apply this approach without DI. But when one sequence of questions use SocialSharing (for example) and another don't - this situation confound me \$\endgroup\$ – Danil Prokhorenko Sep 23 '17 at 7:13
3
\$\begingroup\$

Not the Strategy yet

I'd try to not have a super huge ExtraCard component class if possible. The idea is to encapsulate the data in a Card interface/classes:

export interface Card {
  context: Context;
  question: string;
  answers: Answer[];
}

export interface Answer {
  text: string;
  action: () => void;
}

export class Card1 implements Card {
  constructor(public context: Context) { }

  question = "Some card";
  answers = [
    { text: "Answer 1", action: () => { console.info("Q1-A1"); this.context.ga.doSomething(); } },
    { text: "Answer 2", action: () => { console.info("Q1-A2"); } },
    { text: "Answer 3", action: () => { console.info("Q1-A3"); } }
  ];
}

export class Card2 implements Card {
  constructor(public context: Context) { }

  question = "Some card";
  answers = [
    { text: "Answer 1", action: () => { console.info("Q2-A1"); } },
    { text: "Answer 2", action: () => { console.info("Q2-A2"); } }
  ];
}

Please notice two things here:

  1. the Card is now responsible for holding the data related to the specific Question (both, question and the list of answer with corresponding behavior). This is not really the "Strategy Pattern" yet, but it is a first big step towards it. In Strategy pattern you normally do not mix the data and behavior in the same class, but rather separate them.
  2. We declare a context: Context property that will hold the important dependencies (aka Services) that are required for a given Card to do it's work in answers' actions. This allows the card classes to stay regular TypeScript classes instead of being declared as services. This also means that the Context with the services should be provided from the outside. In TypeScript we can use the constructor (public context: Context) to enforce client pass in the context.

The Context, by the way can look something like this:

export class Context {
  ga: GoogleAnalytics;
  socialSharing: SocialSharing;
  appRateService: AppRateService;
  // Other services/dependencies...
}

Now we need to change the ExtraCard component as well as it consumer.

Why not make the ExctraCard as dumb as we can? Here's what it could look like. Notice that the component now expects the entire Card to be passed in via @Input() card. It will also notify the consumer about receiving an answer (see EventEmitter.

<div *ngIf="isActive">
  <div>{{card.question}}</div>
  <div *ngFor="let answer of card.answers">
    <div (click)="triggerAction(answer)">{{ answer.text }}</div>
  </div>
</div>
import { Component, EventEmitter, Input, Output } from '@angular/core';

@Component({
  selector: 'extra-card',
  templateUrl: 'extra-card.component.html'
})
export class ExtraCard {

  @Input() isActive: boolean = true;
  @Output() answerReceived = new EventEmitter<void>();

  @Input() card: Card;

  triggerAction(answer: Answer): void {
    answer.action();
    this.answerReceived.emit();
  }
}

The last part is your consumer component. It orchestrates Card objects creation and iteration.

The collection of allCards is created in ngOnInit rather than constructor, and it defines the number, kind, and order of the Cards. Consumer's constructor is used to inject the dependencies and prepare a sharedContext that is than passed to the Cards (remember, they now require it in their constructors).

answerReceived() method is used to react to the corresponding event. In my code I assumed that we only need to iterate through the questions linearly (first to last). If that is not the case (e.g. you may need to jump from Card1 to Card3 if a specific Answer was selected), then ExtraCard would need to emit event with some data. For example, answerReceived = new EventEmitter<number>() and this.answerReceived.emit(answer.id). Then the answerId could be used in Consumer for implementing more complex iteration logic.

<div *ngFor="let card of allCards; let cardIndex = index">
  <extra-card
    [isActive]="isActive(card)"
    [question]="card"
    (answerReceived)="answerReceived(cardIndex)"></extra-card>
</div>

import { Component } from '@angular/core';
import { Card1, Card2, Context, Card } from './extra-card.component';

@Component({
  selector: 'consumer',
  templateUrl: 'consumer.component.html'
})
export class Consumer {

    private sharedContext: Context;
    allCards: Card[] = [];
    activeCard: Card;

    constructor(ga: GoogleAnalytics, appRateService: AppRateService, socialSharing: SocialSharing) {
        this.sharedContext = {
            appRateService,
            ga,
            socialSharing
        };
    }

    ngOnInit(): void {
        this.allCards = [
            new Card1(this.sharedContext),
            new Card2(this.sharedContext),
            // ...
        ];

        this.activeCard = this.allCards[0];
    }

    isActive(card: Card): boolean {
        return this.activeCard === card;
    }

    answerReceived(cardIndex: number): void {
        const card = this.allCards[cardIndex + 1];
        if (card != null) {
            this.activeCard = card;
        } else {
            // TODO Handle card enumeration complete event.
        }
    }
}

Disclaimer

This is by no means the best approach I recommend to use generally, but I think it's better what you have now. Especially, since you identified a very important problem of Card/Question quantity growth. My solution is much more scalable, but it too has its limits.

If you apply this for now, and see some other issues coming, feel free to post a follow-up question with a specific ask.

I hope this helps.

\$\endgroup\$
  • \$\begingroup\$ Please don't recommend casting. There is absolutely no reason to cast { text: "Answer 3", action: () => { console.info("Q1-A3"); } } to Answer. By doing so you are telling the compiler to let you shoot you in the foot if you accidentally spelled action as actoin. \$\endgroup\$ – Gerrit0 Sep 29 '17 at 21:57
  • \$\begingroup\$ @Gerrit0 i get what you're saying. This is a minor detail in my code base which is definitely not in the focus of my answer. Nevertheless, could you tell, what is the better concise way to create an object here? \$\endgroup\$ – Igor Soloydenko Sep 29 '17 at 22:52
  • 1
    \$\begingroup\$ Exactly the same way you are already doing it, just remove the casting. So long as answers is already typed the compiler will infer the type. Playground \$\endgroup\$ – Gerrit0 Sep 29 '17 at 23:08
  • \$\begingroup\$ Oh, good. I did not know about that detail. Thank you @Gerrit0! Please check whether the code looks good now. \$\endgroup\$ – Igor Soloydenko Sep 29 '17 at 23:10
  • \$\begingroup\$ It might be somewhat confusing why you use answers instead of this.answers, but other than that looks good to me now. +1. \$\endgroup\$ – Gerrit0 Sep 29 '17 at 23:19
1
\$\begingroup\$

A few things to add..

Igors approach is definitely the right direction but I have a few things to add.

Use interfaces for your models

It makes sense here to separate Card and Answer into interfaces:

export interface QuestionCard {
  context: Context;
  question: string;
  answers: Answer[];
}

export interface Answer {
  text: string;
  action: () => void;
}

Inside your component though, the approach to create classes for every card is inefficient and pointless. That solution is not scalable at all @Igor Soloydenko. Further, unlike classes - interfaces are compile time only and completely removed from code after compilation. See https://jameshenry.blog/typescript-classes-vs-interfaces/

Create a service, even if it is mocked data

Push it a bit and create a service, this is proper and you need it eventually.

@Injectable()
export class CardService() {
    constructor(private http: Http) {}

    getQuestions(): Observable<QuestionCard[]> {
    return  [{
      question: "Question 1?",
      answers: [
          { text: "Answer 1", action: () => { console.info("Q1-A1"); this.context.ga.doSomething(); }},
          { text: "Answer 2", action: () => { console.info("Q1-A2")}},
          { text: "Answer 3", action: () => { console.info("Q1-A3");}}
        ]
      },
      {
        question: "Question 2?",
        answers: [
            { text: "Answer 1", action: () => { console.info("Q1-A1"); this.context.ga.doSomething(); }},
            { text: "Answer 2", action: () => { console.info("Q1-A2")}},
            { text: "Answer 3", action: () => { console.info("Q1-A3");}}
        ]
        }
      ]
  }
 }

Access data from CardService inside your component

@Component({
  selector: 'extra-card',
  templateUrl: 'extra-card.html'
})
export class ExtraCard {
    questions: Question[];
    constructor(private cardService: CardService) {}
    ngOnInit() {
        this.questions = this.cardService.getQuestions();
    }
...
}

Iterate over questions with *ngFor in your template

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.