4
\$\begingroup\$

This my opportunity object which is a part of my angular app.component.ts file. In my template(HTML of this component) I want to loop out different skills backgrounds nationalitiesetc. array using *ngFor*. So I've filtered out these array of objects based on a property option which can have only 2 values either preferred or required. I've implemented such logic inside my TS file using filterByProperty() function as shown below. Please help make this function more simplified and generalized.

opportunity: any = {
 "id": 526,
  "title": "Testing",
  "nationalities": [
    {
      "option": "preferred",
      "id": 479,
      "name": "India",
    }
  ],
  "skills": [
    {
      "option": "required",
      "id": 1153,
      "name": "Adobe Lightroom",
    },
    {
      "option": "preferred",
      "id": 77,
      "name": "Python",
    },
    {
      "option": "preferred",
      "id": 78,
      "name": "Ajax",
    },
    {
      "option": "required",
      "id": 147,
      "name": "3D Max",
    },
    {
      "option": "preferred",
      "id": 175,
      "name": "Aircraft Engine Technology",
    }
  ],
  "backgrounds": [
    {
      "option": "preferred",
      "id": 232,
      "name": "Biology",
    },
    {
      "option": "required",
      "id": 249,
      "name": "International relations",
    }
  ],
  "languages": [
    {
      "option": "required",
      "id": 18,
      "name": "English",
    },
    {
      "option": "preferred",
      "id": 31,
      "name": "Italian",
    }
  ]
}

My app.component.ts file

    import {Component, OnInit, ViewChild} from '@angular/core';

    @Component({
      selector: 'app-root',
      templateUrl: './app.component.html'
    })

    export class AppComponent implements OnInit {

  opportunity: any = {}; // Object value is shown above
  backgrounds: any = {
    preferred: [],
    required: []
  };
  skills: any = {
    preferred: [],
    required: []
  };
  languages: any = {
    preferred: [],
    required: []
  };
  nationalities: any = {
    preferred: [],
    required: []
  };

  ngOnInit() {
      this.filterByProperty();
  }

  filterByProperty() {
    this.skills.preferred = this.opportunity.skills.filter((i) => i.option === 'preferred');
    this.skills.required = this.opportunity.skills.filter((i) => i.option === 'required');
    this.backgrounds.preferred = this.opportunity.backgrounds.filter((i) => i.option === 'preferred');
    this.backgrounds.required = this.opportunity.backgrounds.filter((i) => i.option === 'required');
    this.languages.preferred = this.opportunity.languages.filter((i) => i.option === 'preferred');
    this.languages.required = this.opportunity.languages.filter((i) => i.option === 'required');
    this.nationalities.preferred = this.opportunity.nationalities.filter((i) => i.option === 'preferred');
    this.nationalities.required = this.opportunity.nationalities.filter((i) => i.option === 'required');
  }
}
\$\endgroup\$
  • \$\begingroup\$ I am not going to post an answer, because I don't have experience with angular, but why not parse opportunities once by iterating over its keys? \$\endgroup\$ – Sumurai8 Apr 29 '18 at 11:14
  • \$\begingroup\$ You can definitely answer the question because I'm just asking logic.. which will be written in javascript. \$\endgroup\$ – Yashwardhan Pauranik Apr 29 '18 at 11:24
2
\$\begingroup\$

The most important piece of advice I can offer is get rid of any. By using any you are making Typescript ignore potential errors in your program. opportunity could be described with the following interfaces:

interface Opportunity {
  id: number
  title: string
  nationalities: Criteria[]
  skills: Criteria[]
  backgrounds: Criteria[]
  languages: Criteria[]
}

interface Criteria {
  option: 'preferred' | 'required'
  id: number
  name: string
}

Furthermore, the backgrounds, skills, languages, and nationalities properties should be correctly typed. In this case, I'd just add another interface, SeparatedCriteria isn't really the best name, but you can probably come up with a better one as you are more familiar with what each piece of data represents.

interface SeparatedCriteria {
  preferred: Criteria[]
  required: Criteria[]
}

filterByProperty has a lot of redundancy. At the very least, I would recommend defining isRequired and isPreferred local functions to help cut down on some of the noise.

However, this is still rather noisy especially as the logic is the same for each property - the only thing that changes is the key. With this in mind, we can define an array of keys, ['skills', 'backgrounds', 'languages', 'nationalities'] and assign this[key].preferred and this[key].required.

The trick to making this work correctly is to use both intersection types and index types. It is possible to do this without knowledge of these topics, but the solution will not be type safe as it would be possible to add keys which aren't present in both Opportunity and AppComponent without receiving a compiler error. Here's how I would implement this function. You could now get rid of isRequired and isPreferred as they are only used once, but I believe it makes the logic more clear to leave them as is.

filterByProperty() {
  const isPreferred = ({ option }: Criteria) => option == 'preferred';
  const isRequired = ({ option }: Criteria) => option == 'required';

  const propertyKeys: Array<keyof AppComponent & keyof Opportunity>
    = ['skills', 'backgrounds', 'languages', 'nationalities'];

  for (const key of propertyKeys) {
    this[key].preferred = this.opportunity[key].filter(isPreferred);
    this[key].required = this.opportunity[key].filter(isRequired);
  }
}
\$\endgroup\$
  • \$\begingroup\$ Thanks for helping me out. And I'll surely implement the recommended changes. \$\endgroup\$ – Yashwardhan Pauranik Apr 30 '18 at 4:40
  • \$\begingroup\$ Can you explain what does that mean in simple english. Array<keyof AppComponent & keyof Opportunity> \$\endgroup\$ – Yashwardhan Pauranik May 17 '18 at 14:14
  • 1
    \$\begingroup\$ keyof AppComponent gives an intersection type of all the keys of AppComponent, this is all the property and method names on the class instance. keyof Opportunity gives "id" | "title" | "nationalities" | "skills" | "backgrounds" | "languages", using & to combine them ensures you can only have keys which are contained in *both*Opportunity and AppComponent, then we just define an array of those keys. \$\endgroup\$ – Gerrit0 May 18 '18 at 0:08
  • \$\begingroup\$ Got it man.. well explained 😀 \$\endgroup\$ – Yashwardhan Pauranik May 18 '18 at 7:26

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.