3
\$\begingroup\$

As a front-end developer, I am given the following result from an API, an array of objects that looks like:

[{
    Color: "#065b79",
    GroupName: "VXYZ",
    QuoteCount: 4,
    SortOrder: 0,
    TabCategoryID: "CHSI1",
    TabCategoryName: "Workers' Compensation",
    TabCategoryOrder: 0,
    TabID: 1,
    TabName: "new"
}]

I need to display the data in a more hierarchical/nested format:

[{
    GroupName: 'VXYZ',
    TabCategories: [
        {
            TabCategoryID: 'CHSI1',
            TabCategoryName: "Workers' Compensation",
            TabCategoryOrder: 0,
            Tabs: [
                {
                    Color: '#065b79',
                    SortOrder: 0,
                    TabID: 1,
                    TabName: 'new',
                    QuoteCount: 4
                }
            ]
        }
    ]
}]

Any data in an array can have any number of array items. In the models above, I just use a single instance of each array item.

This is the code I used to format the data

I'm using Typescript and lodash. The API is experimental, and our policy is to just use the any type for any data coming from experimental APIs. This explains the (Quotes: any) — it is the data from the API.

public FormatQuoteSummaries(Quotes: any): any {
    
    return _(Quotes)
    .map(Quote => Quote.GroupName)
    .uniq()
    .map(UniqueGroupName => {
        const GroupName = UniqueGroupName
        const AllTabCategories = _.filter(Quotes, (Quote: any) => {
            return Quote.GroupName === UniqueGroupName
        })
        const TabCategories = _(AllTabCategories).map(TabCategory => {
            return {
                TabCategoryID: TabCategory.TabCategoryID,
                TabCategoryName: TabCategory.TabCategoryName,
                Tabs: _.filter(Quotes, (Quote: any) => {
                    return Quote.TabCategoryName === TabCategory.TabCategoryName &&
                        Quote.GroupName === TabCategory.GroupName
                })
            }
        })
            .uniqBy('TabCategoryID')
            .value();

        return {
            GroupName,
            TabCategories
        }
    })
    .value();
}
\$\endgroup\$
3
\$\begingroup\$

First and most importantly, change your policy about using any for unstable APIs.

Typescript is an incredibly powerful tool for making it possible to work with constantly changing APIs. When you use any you are throwing away (nearly) all the benefits which Typescript gives you. If you declare an interface for the data being returned by the API, you can simply modify the interface when the format changes and you will be alerted to any code which needs to be updated. If you instead use any your code will compile without any warnings - and then break in production.

Besides this, there really isn't that much to mention. Good work!

On to the code:

  1. The first thing I did was add types, here are the interfaces I will be using for the rest of the answer:

    interface APITabCategory {
        Color: string;
        GroupName: string;
        QuoteCount: number;
        SortOrder: number;
        TabCategoryID: string;
        TabCategoryName: string;
        TabCategoryOrder: number;
        TabID: number;
        TabName: string;
    }
    
    interface Tab {
        Color: string;
        SortOrder: number;
        TabID: number;
        TabName: string;
        QuoteCount: number;
    }
    
    interface TabCategory {
        TabCategoryID: string;
        TabCategoryName: string;
        TabCategoryOrder: number;
        Tabs: Tab[]
    }
    
    interface TabCategoryCollection {
        GroupName: string;
        TabCategories: TabCategory[]
    }
    
  2. It doesn't do what you think it does. As soon as I added interfaces for your types Typescript yelled at me that TabCategoryOrder was missing - this may be intentional as you are now using an array, but it is included in your sample output.

  3. The current code includes a bunch of extra data in each Tab, this is fine so far as Typescript is concerned, but since this code normalizes the data you may want to consider removing the extra properties.

  4. JavaScript has built in map and filter functions on arrays, I prefer using these to their lodash alternatives where appropriate. In this case, it removes most of your code's dependence on lodash.

  5. Avoid double checking conditions. If you save the quotes with the current group name in an array, you can avoid checking the group name when extracting the tabs.

  6. Get a linting program and use it - the current code is inconsistent with whether or not to use semicolons.

Here is how I would implement this function:

function FormatQuoteSummaries(Quotes: APITabCategory[]): TabCategoryCollection[] {
    const UniqueGroupNames = _.uniq(Quotes.map(Quote => Quote.GroupName));

    return UniqueGroupNames.map(GroupName => {
        const GroupQuotes = Quotes.filter(Quote => Quote.GroupName == GroupName)

        const TabCategories: TabCategory[] = GroupQuotes
            .filter(Quote => Quote.GroupName === GroupName)
            .map(({ TabCategoryID, TabCategoryName, TabCategoryOrder }) => {
                return {
                    TabCategoryID,
                    TabCategoryName,
                    TabCategoryOrder,
                    Tabs: GroupQuotes.filter(Quote => Quote.TabCategoryName === TabCategoryName)
                };
            });

        return {
            GroupName,
            TabCategories: _.uniqBy(TabCategories, 'TabCategoryID')
        };
    });
}

One last small nitpick, it is fairly standard is JavaScript / TypeScript to use camelCase as opposed to PascalCase for local variables and properties. Most JS / TS devs, when seeing Quotes expect a class / interface / enum / namespace, not an object or array. Feel free to disregard if your team has an established naming convention already.

\$\endgroup\$
  • 1
    \$\begingroup\$ I find your second paragraph somewhat odd and bordering on misleading, particularly the words "immediately discovering incompatibilities", (A) this assumes you are constantly testing production code, not a very practical or affordable way to run a service. (B) type does not protect against data format, ie temperature as a number makes no distinction to the scale, Kelvin, Celsius, or Fahrenheit. Maybe "immediately discovering incompatibilities" would be better as "simplifies the process of discovering incompatibilities" \$\endgroup\$ – Blindman67 Apr 3 '18 at 10:16
  • \$\begingroup\$ @Blindman67 fair point, I decided the rest of the paragraph spoke for itself and just decided to remove that clause. \$\endgroup\$ – Gerrit0 Apr 3 '18 at 18:55

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.