Possible way to combine to methods that use duplicate code

Question

I have two methods getTargetComponentById and getTargetComponentByName which perform a lookup on an object based on a particular key and returns an object. The two methods work fine seperated, but I would like to combine them to cut down on duplicated code. What would be the best way of combining them:

  getTargetComponentById(componentId: string): IComponentTarget[] {
    const targetComponents = [];
    const website = this.website.getValue();
    for (let i = 0; i < website.pages.length; i++) {
      for (let j = 0; j < website.pages[i].components.length; j++) {
        if (website.pages[i].components[j].componentId === componentId) {
          targetComponents.push({
            activePageIndex: i,
            activeComponentIndex: j,
          });
        }
      }
    }
    return targetComponents;
  }

  getTargetComponentByName(componentName: string, activeWebsite = null): IComponentTarget[] {
    let website: IWebsite;
    if (activeWebsite === null) {
      website = this.website.getValue();
    } else {
      website = activeWebsite;
    }
    const targetComponents = [];
    for (let i = 0; i < website.pages.length; i++) {
      for (let j = 0; j < website.pages[i].components.length; j++) {
        if (website.pages[i].components[j].componentName === componentName) {
          targetComponents.push({
            activePageIndex: i,
            activeComponentIndex: j,
          });
        }
      }
    }
    return targetComponents;
  }

getTargetComponentById(componentId: string, activeWebsite = null), const website = activeWebsite || this.website.getValue();, then you only have to find a way how to distinguish an id from a name. — Andreas
– Andreas, Commented Jun 4, 2020 at 8:50

ABCoder · Accepted Answer · 2020-06-04 09:05:22Z

1

You could declare the property to use as an extra input parameter.

getTargetComponentByProperty(property: 'componentName' | 'componentId', value: string, activeWebsite ?= null): IComponentTarget[] {
  const website: IWebsite = activeWebsite ? activeWebsite : this.website.getValue();
  const targetComponents = [];
  for (let i = 0; i < website.pages.length; i++) {
    for (let j = 0; j < website.pages[i].components.length; j++) {
      if (website.pages[i].components[j][property] === value) {
        targetComponents.push({
          activePageIndex: i,
          activeComponentIndex: j,
        });
      }
    }
  }
  return targetComponents;
}

answered Jun 4, 2020 at 9:05

ABCoder

514 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Eliseo · Accepted Answer · 2020-06-04 08:56:32Z

1

I like this

getTargetComponent(arg: string|number, activeWebsite ?= null): IComponentTarget[] {
  const website = activeWebsite || this.website.getValue();
  if (''+arg===arg)
      console.log("arg is a string")
  else
      console.log("arg is a number")

 //well, you can use
 const field=(''+arg===arg)?'componentName':'componentId'
 //and use components[j][field]==arg
}

answered Jun 4, 2020 at 8:56

Eliseo

58.9k4 gold badges39 silver badges82 bronze badges

Comments

Barremian · Accepted Answer · 2020-06-04 08:59:02Z

1

You could use bracket notation instead of dot notation property accessor to generalize the statement website.pages[i].components[j].componentName === componentName. Try the following

getTargetComponent(component: string, activeWebsite ?= null): IComponentTarget[] {
  const website: IWebsite = activeWebsite ? activeWebsite : this.website.getValue();
  const targetComponents = [];
  for (let i = 0; i < website.pages.length; i++) {
    for (let j = 0; j < website.pages[i].components.length; j++) {
      if (website.pages[i].components[j][component] === component) {        // <-- bracket notation here
        targetComponents.push({
          activePageIndex: i,
          activeComponentIndex: j,
        });
      }
    }
  }
  return targetComponents;
}

edited Jun 4, 2020 at 8:59

answered Jun 4, 2020 at 8:53

Barremian

31.2k4 gold badges37 silver badges67 bronze badges

Comments

Sandman · Accepted Answer · 2020-06-04 09:10:14Z

You can expose two different interface methods that internally use only one method for logic. I think this is the most elegant way of doing it. If you are using angular (and therefore typescript) you can also enter the two methods as public and the core method as private.

  public getTargetComponentById(id: string): IComponentTarget[] {
    return getTargetComponentBy(id, null, 'componentId');
  }

  public getTargetComponentByName(name: string, activeWebsite = null): IComponentTarget[] {
    return getTargetComponentBy(name, activeWebsite, 'componentName');
  }

  private getTargetComponentBy(componentKey: string, activeWebsite = null, prop: string): IComponentTarget[] {
    let website: IWebsite;
    if (activeWebsite === null) {
      website = this.website.getValue();
    } else {
      website = activeWebsite;
    }
    const targetComponents = [];
    for (let i = 0; i < website.pages.length; i++) {
      for (let j = 0; j < website.pages[i].components.length; j++) {
        const aComp = website.pages[i].components[j];
        if (website.pages[i].components[j][prop] === componentKey) {
          targetComponents.push({
            activePageIndex: i,
            activeComponentIndex: j,
          });
        }
      }
    }
    return targetComponents;
  }

Collectives™ on Stack Overflow

Possible way to combine to methods that use duplicate code

4 Answers 4

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related