search

cap-js / ord

Open Resource Discovery (ORD) is a protocol that allows applications and services to self-describe their exposed resources and capabilities. This plugin enables generation of ORD document for CAP based applications.
Apache License 2.0
3 stars 4 forks source link

Fix: replace global variable and refactor code #71

Closed zongqichen closed 1 month ago

aramovic79 commented 1 month ago

Putting two cents:

enums vs objects in javascript

You can find on many places that javascript developers prefer using Object.freeze instead of enum (or as const casting).

Enum can be defined as:

    const RESOURCE_TYPE = Object.freeze(
    {
        "api": "api",
        "event": "event"
    });

Please pay attention on Object.freeze. It serves to disallow changing of the object which at the end causes that RESOURCE_TYPE acts as an enum.

aramovic79 commented 1 month ago

I think that the code: 

function _getPackageID(namespace, packageIds, apiOrEvent) {
    if (packageIds instanceof Set) {
        const packageArray = Array.from(packageIds);

        if (apiOrEvent === "api") {
            const apiPackage = packageArray.find((pkg) => pkg.includes("-api"));
            if (apiPackage) return apiPackage;
        } else if (apiOrEvent === "event") {
            const eventPackage = packageArray.find((pkg) => pkg.includes("-event"));
            if (eventPackage) return eventPackage;
        }

        return packageArray.find(pkg => pkg.includes(namespace));
    }
}

can be written this way:

function _getPackageID(namespace, packageIds, apiOrEvent) {
    return packageIds.find((pkg) => pkg.includes("-" + apiOrEvent)) || packageIds.find((pkg) => pkg.includes(namespace));
}

where I think that instead of apiOrEvent we should name this input parameter like resourceType

zongqichen commented 1 month ago

I think that the code:

function _getPackageID(namespace, packageIds, apiOrEvent) {
    if (packageIds instanceof Set) {
        const packageArray = Array.from(packageIds);

        if (apiOrEvent === "api") {
            const apiPackage = packageArray.find((pkg) => pkg.includes("-api"));
            if (apiPackage) return apiPackage;
        } else if (apiOrEvent === "event") {
            const eventPackage = packageArray.find((pkg) => pkg.includes("-event"));
            if (eventPackage) return eventPackage;
        }

        return packageArray.find(pkg => pkg.includes(namespace));
    }
}

can be written this way:

function _getPackageID(namespace, packageIds, apiOrEvent) {
    return packageIds.find((pkg) => pkg.includes("-" + apiOrEvent)) || packageIds.find((pkg) => pkg.includes(namespace));
}

where I think that instead of apiOrEvent we should name this input parameter like resourceType

I create an issue regarding to this refactor: https://github.com/cap-js/ord/issues/73