Pergunta

I was looking at the angular documentation and noticed this code:

export class QuestionBase<T> {
  value: T;
  key: string;
  label: string;
  required: boolean;
  order: number;
  controlType: string;
  type: string;
  options: {key: string, value: string}[];

  constructor(options: {
      value?: T,
      key?: string,
      label?: string,
      required?: boolean,
      order?: number,
      controlType?: string,
      type?: string
    } = {}) {
    this.value = options.value;
    this.key = options.key || '';
    this.label = options.label || '';
    this.required = !!options.required;
    this.order = options.order === undefined ? 1 : options.order;
    this.controlType = options.controlType || '';
    this.type = options.type || '';
  }
}

They use this as the base class for different types of questions such as "DropdownQuestion", "TextboxQuestion", etc. I've copied the usage they included in the docs below:

export class DropdownQuestion extends QuestionBase<string> {
  controlType = 'dropdown';
  options: {key: string, value: string}[] = [];

  constructor(options: {} = {}) {
    super(options);
    this.options = options['options'] || [];
  }
}
 //in some other file...
  new DropdownQuestion({
    key: 'brave',
    label: 'Bravery Rating',
    options: [
      {key: 'solid',  value: 'Solid'},
      {key: 'great',  value: 'Great'},
      {key: 'good',   value: 'Good'},
      {key: 'unproven', value: 'Unproven'}
    ],
    order: 3
  }),

  new TextboxQuestion({
    key: 'firstName',
    label: 'First name',
    value: 'Bombasto',
    required: true,
    order: 1
  }),

  new TextboxQuestion({
    key: 'emailAddress',
    label: 'Email',
    type: 'email',
    order: 2
  })

As you can see, only the DropdownQuestion uses the "options" property, and has it's own property for "options", but they've included it in the base class. From what I can tell, it literally does nothing in the base class, because it's not used in the constructor, and it will never reach the prototype. What is the point of doing that? Shouldn't it just be included in the DropdownQuestion?

Slightly unrelated, but making the "key" and "label" properties optional when they seem to be required in all questions also seems like an odd thing to do.

Can anyone provide some clarification on what is ideal to do?

Foi útil?

Solução

Having a property in the base class which is used in only one of several subclasses is a common antipattern. This can easily happen when creating the superclass, by either pulling too many things into the superclass when creating it or forgetting to pull the property into the new subclass (depending on how the initial refactoring happened).

Better naming might have made this flaw clearer. In this case, the "dropdown" part of DropdownQuestion is misleading: a question with options might be displayed as a single-select list (aka "dropdown"), but might just as easily be a multi-select or a set of radio buttons. If it were instead named (for example) SingleOptionQuestion that would make it obvious that the options property should be in this class.

Outras dicas

Including any public properties in base class is an anti pattern to begin with, or at least it's a code smell. In most cases, you should follow the tell don't ask principle, and the matters of property wouldn't even become an issue in the first place.

In this particular case, the options could simply be a matter of render(), getDefault() and validate() method, which should remove the need to actually have options to be exposed as a property.

Licenciado em: CC-BY-SA com atribuição
scroll top