Factory method done on ReactJS component

Question

A Factory Pattern or Factory Method Pattern says that just define an interface or abstract class for creating an object but let the subclasses decide which class to instantiate. In other words, subclasses are responsible to create the instance of the class.

Advantage of Factory Design Pattern

Factory Method Pattern allows the sub-classes to choose the type of objects to create. It promotes the loose-coupling by eliminating the need to bind application-specific classes into the code. That means the code interacts solely with the resultant interface or abstract class, so that it will work with any classes that implement that interface or that extends that abstract class. A factory method is a method of a factory that builds objects.

With that said, I built a React component that uses this pattern, functionally it works perfect, but I would like to know if the pattern is properly implemented or not and what your feedback as an expert is on this scenario.

ListItemFactory.ts

import { SPHttpClient, SPHttpClientResponse } from "@microsoft/sp-http";
import { IWebPartContext } from "@microsoft/sp-webpart-base";
import { IListItem} from "./models/IListItem";
import { IFactory } from "./IFactory";
import { INewsListItem } from "./models/INewsListItem";
import { IDirectoryListItem } from "./models/IDirectoryListItem";
import { IAnnouncementListItem } from "./models/IAnnouncementListItem";

export class ListItemFactory implements IFactory {
    // private _listItems: IListItem[];
    public getItems(requester: SPHttpClient, siteUrl: string, listName: string): Promise<any[]> {
        switch(listName) {
            case "GenericList":
                let items: IListItem[];
                // tslint:disable-next-line:max-line-length
                return requester.get(`${siteUrl}/_api/web/lists/getbytitle('${listName}')/items?$select=Title,Id,Modified,Created,Author/Title,Editor/Title&$expand=Author,Editor`,
                SPHttpClient.configurations.v1,
                {
                    headers: {
                        "Accept": "application/json;odata=nometadata",
                        "odata-version": ""
                    }
                })
                .then((response: SPHttpClientResponse): Promise<{ value: IListItem[] }> => {
                    return response.json();
                })
                .then((json: { value: IListItem[] }) => {
                    console.log(JSON.stringify(json.value));
                    return items=json.value.map((v,i)=>(
                        {
                            // key: v.id,
                            id: v.Id,
                            title: v.Title,
                            created: v.Created,
                            createdby: v.Author.Title,
                            modified: v.Modified,
                            modifiedby: v.Editor.Title
                        }
                    ));
                });
            case "News":
                let newsitems: INewsListItem[];
                // tslint:disable-next-line:max-line-length
                return requester.get(`${siteUrl}/_api/web/lists/getbytitle('${listName}')/items?$select=Title,Id,Modified,Created,Created By,Modified By,newsheader,newsbody,expiryDate`,
                SPHttpClient.configurations.v1,
                {
                    headers: {
                        "Accept": "application/json;odata=nometadata",
                        "odata-version": ""
                    }
                })
                .then((response: SPHttpClientResponse): Promise<{ value: INewsListItem[] }> => {
                    return response.json();
                })
                .then((json: { value: INewsListItem[] }) => {
                    return newsitems=json.value.map((v,i)=>(
                        { 
                            id: v.Id,
                            title: v.Title,
                            created: v.Created,
                            createdby: v.Author.Title,
                            modified: v.Modified,
                            modifiedby: v.Editor.Title,
                            newsheader: v.newsheader,
                            newsbody: v.newsbody,
                            expiryDate: v.expiryDate
                        }
                    ));
                });
            case "Announcements":
                let announcementitems: IAnnouncementListItem[];
                return requester.get(`${siteUrl}/_api/web/lists/getbytitle('${listName}')/items?$select=Title,Id,Created,Author/Title,Modified,Editor/Title,announcementBody,expiryDate&$expand=Author,Editor`,
                SPHttpClient.configurations.v1,
                {
                    headers: {
                        "Accept": "application/json;odata=nometadata",
                        "odata-version": ""
                    }
                })
                .then((response: SPHttpClientResponse): Promise<{ value: IAnnouncementListItem[] }> => {
                    return response.json();
                })
                .then((json: { value: IAnnouncementListItem[] }) => {
                    return announcementitems=json.value.map((v,i)=>(
                        { 
                            id: v.Id,
                            title: v.Title,
                            created: v.Created,
                            createdby: v.Author.Title,
                            modified: v.Modified,
                            modifiedby: v.Editor.Title,
                            announcementBody: v.announcementBody,
                            expiryDate: v.expiryDate
                        }
                    ));
                });
            case "Directory":
                let directoryitems: IDirectoryListItem[];
                return requester.get(`${siteUrl}/_api/web/lists/getbytitle('${listName}')/items?$select=Title,Id`,
                SPHttpClient.configurations.v1,
                {
                    headers: {
                        "Accept": "application/json;odata=nometadata",
                        "odata-version": ""
                    }
                })
                .then((response: SPHttpClientResponse): Promise<{ value: IDirectoryListItem[] }> => {
                    return response.json();
                })
                .then((json: { value: IDirectoryListItem[] }) => {
                    return directoryitems=json.value.map((v,i)=>(
                        {
                            id: v.Id,
                            title: v.Title,
                            created: v.Created,
                            createdby: v.Author.Title,
                            modified: v.Modified,
                            modifiedby: v.Editor.Title,
                            firstName: v.firstName,
                            lastName: v.lastName,
                            mobileNumber: v.mobileNumber,
                            internalNumber: v.internalNumber
                        }
                    ));
                });
            default:
                break;
            }
      }
}

Ifactorystate.ts

import { IListItem } from "./models/IListItem";
import { INewsListItem } from "./models/INewsListItem";
import { IDirectoryListItem } from "./models/IDirectoryListItem";
import { IAnnouncementListItem } from "./models/IAnnouncementListItem";
import {
  IColumn
} from "office-ui-fabric-react/lib/DetailsList";

export interface IFactoryMethodState {
  hasError: boolean;
  status: string;
  columns: IColumn[];
  DetailsListItemState: IDetailsListItemState;
  DetailsNewsListItemState: IDetailsNewsListItemState;
  DetailsDirectoryListItemState : IDetailsDirectoryListItemState;
  DetailsAnnouncementsListItemState : IDetailsAnnouncementListItemState;
}

export interface IDetailsListItemState {
  items: IListItem[];
}

export interface IDetailsNewsListItemState {
  items: INewsListItem[];
}

export interface IDetailsDirectoryListItemState {
  items: IDirectoryListItem[];
}

export interface IDetailsAnnouncementListItemState {
  items: IAnnouncementListItem[];
}

and the component

//#region Imports
import * as React from "react";
import styles from "./FactoryMethod.module.scss";
import  { IFactoryMethodProps } from "./IFactoryMethodProps";
import {
  IDetailsListItemState,
  IDetailsNewsListItemState,
  IDetailsDirectoryListItemState,
  IDetailsAnnouncementListItemState,
  IFactoryMethodState
} from "./IFactoryMethodState";
import { IListItem } from "./models/IListItem";
import { IAnnouncementListItem } from "./models/IAnnouncementListItem";
import { INewsListItem } from "./models/INewsListItem";
import { IDirectoryListItem } from "./models/IDirectoryListItem";
import { escape } from "@microsoft/sp-lodash-subset";
import { SPHttpClient, SPHttpClientResponse } from "@microsoft/sp-http";
import { ListItemFactory} from "./ListItemFactory";
import { TextField } from "office-ui-fabric-react/lib/TextField";
import {
  DetailsList,
  DetailsListLayoutMode,
  Selection,
  buildColumns,
  IColumn
} from "office-ui-fabric-react/lib/DetailsList";
import { MarqueeSelection } from "office-ui-fabric-react/lib/MarqueeSelection";
import { autobind } from "office-ui-fabric-react/lib/Utilities";
import PropTypes from "prop-types";
//#endregion

export default class FactoryMethod extends React.Component<IFactoryMethodProps, IFactoryMethodState> {
  constructor(props: IFactoryMethodProps, state: any) {
    super(props);
    this.setInitialState();
  }


  // lifecycle help here: https://staminaloops.github.io/undefinedisnotafunction/understanding-react/
  //#region Mouting events lifecycle
  // the data returned from render is neither a string nor a DOM node.
  // it's a lightweight description of what the DOM should look like.
  // inspects this.state and this.props and create the markup.
  // when your data changes, the render method is called again.
  // react diff the return value from the previous call to render with
  // the new one, and generate a minimal set of changes to be applied to the DOM.
  public render(): React.ReactElement<IFactoryMethodProps> {
    if (this.state.hasError) {
      // you can render any custom fallback UI
      return <h1>Something went wrong.</h1>;
    } else {
      switch(this.props.listName) {
          case "GenericList":
            // tslint:disable-next-line:max-line-length
            return <this.ListMarqueeSelection items={this.state.DetailsListItemState.items} columns={this.state.columns} />;
          case "News":
            // tslint:disable-next-line:max-line-length
            return <this.ListMarqueeSelection items={this.state.DetailsNewsListItemState.items} columns={this.state.columns}/>;
          case "Announcements":
            // tslint:disable-next-line:max-line-length
            return <this.ListMarqueeSelection items={this.state.DetailsAnnouncementsListItemState.items} columns={this.state.columns}/>;
          case "Directory":
            // tslint:disable-next-line:max-line-length
            return <this.ListMarqueeSelection items={this.state.DetailsDirectoryListItemState.items} columns={this.state.columns}/>;
          default:
            return null;
      }
    }
  }

  public componentDidCatch(error: any, info: any): void {
    // display fallback UI
    this.setState({ hasError: true });
    // you can also log the error to an error reporting service
    console.log(error);
    console.log(info);
  }



  // componentDidMount() is invoked immediately after a component is mounted. Initialization that requires DOM nodes should go here.
  // if you need to load data from a remote endpoint, this is a good place to instantiate the network request.
  // this method is a good place to set up any subscriptions. If you do that, don’t forget to unsubscribe in componentWillUnmount().
  // calling setState() in this method will trigger an extra rendering, but it is guaranteed to flush during the same tick.
  // this guarantees that even though the render() will be called twice in this case, the user won’t see the intermediate state.
  // use this pattern with caution because it often causes performance issues. It can, however, be necessary for cases like modals and
  // tooltips when you need to measure a DOM node before rendering something that depends on its size or position.
   public componentDidMount(): void {
    this._configureWebPart = this._configureWebPart.bind(this);
    this.readItemsAndSetStatus(this.props.listName);
  }

  //#endregion
  //#region Props changes lifecycle events (after a property changes from parent component)
  // componentWillReceiveProps() is invoked before a mounted component receives new props.
  // if you need to update the state in response to prop
  // changes (for example, to reset it), you may compare this.props and nextProps and perform state transitions
  // using this.setState() in this method.
  // note that React may call this method even if the props have not changed, so make sure to compare the current
  // and next values if you only want to handle changes.
  // this may occur when the parent component causes your component to re-render.
  // react doesn’t call componentWillReceiveProps() with initial props during mounting. It only calls this
  // method if some of component’s props may update
  // calling this.setState() generally doesn’t trigger componentWillReceiveProps()
  public componentWillReceiveProps(nextProps: IFactoryMethodProps): void {
    if(nextProps.listName !== this.props.listName) {
      this.readItemsAndSetStatus(nextProps.listName);
    }
  }

  //#endregion
  //#region private methods
  private _configureWebPart(): void {
    this.props.configureStartCallback();
  }

  public setInitialState(): void {
    this.state = {
      hasError: false,
      status: this.listNotConfigured(this.props)
        ? "Please configure list in Web Part properties"
        : "Ready",
      columns:[],
      DetailsListItemState:{
        items:[]
      },
      DetailsNewsListItemState:{
        items:[]
      },
      DetailsDirectoryListItemState:{
        items:[]
      },
      DetailsAnnouncementsListItemState:{
        items:[]
      },
    };
  }

  // reusable inline component
  private ListMarqueeSelection = (itemState: {columns: IColumn[], items: IListItem[] }) => (
      <div>
          <DetailsList
            items={ itemState.items }
            columns={ itemState.columns }
            setKey="set"
            layoutMode={ DetailsListLayoutMode.fixedColumns }
            selectionPreservedOnEmptyClick={ true }
            compact={ true }>
          </DetailsList>
      </div>
  )

  // read items using factory method pattern and sets state accordingly
  private readItemsAndSetStatus(listName): void {
    this.setState({
      status: "Loading all items..."
    });

    const factory: ListItemFactory = new ListItemFactory();
    factory.getItems(this.props.spHttpClient, this.props.siteUrl, listName || this.props.listName)
    .then((items: any[]) => {

      var myItems: any = null;
      switch(this.props.listName) {
          case "GenericList":
              items = items as IListItem[];
              break;
          case "News":
              items = items as INewsListItem[];
              break;
          case "Announcements":
              items = items as IAnnouncementListItem[];
              break;
          case "Directory":
              items = items as IDirectoryListItem[];
              break;
      }

      const keyPart: string = this.props.listName === "GenericList" ? "" : this.props.listName;
        // the explicit specification of the type argument `keyof {}` is bad and
        // it should not be required.
        this.setState<keyof {}>({
          status: `Successfully loaded ${items.length} items`,
          ["Details" + keyPart + "ListItemState"] : {
            items
          },
          columns: buildColumns(items)
        });
    });
  }

  private listNotConfigured(props: IFactoryMethodProps): boolean {
    return props.listName === undefined ||
      props.listName === null ||
      props.listName.length === 0;
  }

  //#endregion
}

Answer 1

When I feel a design is overcomplicated (read: "big objects/functions"), I take a step back and think about the concerns. You have multiple objects and files, so which one should you start with? I always try to start from the top – the largest thing, the thing highest in the call stack, and/or the thing closest to the user – this approach reduces (or at least highlights) complexity faster than starting from the bottom up. This would be your component.

Here's all the things your component does:

• Use the factory to get data
• Use additional switch statements based on the type of things returned from ListItemFactory to do not much
• Render a list given a set of (ideally) interchangeable items.

The multiple switch statements with the same cases are a smell, and in this case it looks like you've built your component to use concrete types, when it should be using some shared interface (abstract type). This is the whole reason for using factories and duck-typed objects (aka Liskov substitution principle, the L in SOLID). You shouldn't have to worry about the concrete type, only that everything shares the same behavior – in this case these things must all return state that's compatible with ListMarqueeSelection's items prop: IListItem, INewsListItem, IAnnouncementListItem, IDirectoryListItem. Use TypeScript to enforce that if you want, but omit mentions of the concrete classes in your component.

To explain more fully, if you're using a factory, you shouldn't see any references to the underlying object types (e.g. IListItem) after calling the factory; otherwise, you're going to have to update the component anytime you want to add a new object type, and this is a violation of the Open-closed principle (the O in SOLID). You should be able to add new types of lists without modifying the component – only the factory. In your case, it's probably as simple as just removing the switch statement, and storing items with setState(). Here I feel like TypeScript is working against you, and if you omit the type casting, your code gets cleaner.

I would write your component to look more like this:

import listFactory from 'wherever';

const MarqueeThing = ({ items }) => (
  <ul>
    {items.map(item => (<li>{item.text}</li>))}
  </ul>
);

class List extends Component {
  constructor(props) {
    super(props);
    this.state = { items: [] }
  }
  
  componentDidMount() {
    const listFetcher = listFactory(this.props.listType)
    listFetcher.fetch().then(items => this.setState({ items });
  }
  
  render() {
    if (!this.state.items.length) {
      return 'Loading...';
    }
    
    return (<MarqueeThing items={this.state.items} />);
  }
}

On to the factory – here's all the things your ListItemFactory does:

• Select what kind of thing to return (the switch-statement)
• Fetch data
• Transform data

Factories typically just do the first thing – they may or may not actually instantiate classes (often they just return the class type), and they certainly don't have intimate details about the construction of the things they return.

I would start by moving the code in each switch statement to a separate object (you already have these – IListItem, INewsListItem, IAnnouncementListItem, IDirectoryListItem). After doing so, the factory can just return the object type, and the switch statement can simply be replaced with a dictionary. The component that uses the factory's returned object type can then instantiate/invoke.

I would rewrite your factory to look more like this:

import newsItemFetcher from 'wherever';
import directoryItemsFetcher from 'wherever';

export default (listType) => {
  const lookup = {
    'News': newsItemsFetcher,
    'Directory': directoryItemsFetcher,
    // ...
  }
  return lookup[listType];
}

Then, you just need to make sure each of the items returned from the factory has a .fetch() that returns a Promise.

Finally, on to the concrete objects returned from the factory. At this point, these are data providers masquerading as models. They're functions and transformations, and they return data in the same format, but it's the transformation that's important, not the data. This is a common problem people have with OOP - they focus too much on the object's properties, and not enough on the messages (function calls / behavior). Alan Kay, the guy that coined the term OOP regrets the name for this reason.

With that in mind, I would update all of your I*Item things to simply be a function that returns a promise, and this is the code that's in the factory's switch statement. Then, you could remove the .fetch() in the component, and simply do:

const fetch = listFetcherFactory(this.props.listType);
fetch().then(items => this.setState({ items });

Please excuse my rough code - if you need clarification, please ask.