The following code should return an array (flatFeatures) which will include feature.name and the most far subscription.expDate. for example if I have some subscriptions with the same features, the flatFeatures array will include distinct of all features from all the subscriptions but for every feature we will have the most later date of the subscription it belongs to. Something like that in SQL:
SELECT Features.Name, MAX(subscriptions.expDate)
FROM Subscriptions
INNER JOIN Plans ON Subscriptions.planId = Plans.planId
INNER JOIN Features ON Plans.PlanId = Features.PlanId
But i need it to be done by NodeJS for the JSON i already have from MongoDB.
const flatFeatures = new Array();
allActiveSubscriptions.forEach(subscription => {
if (subscription.plan && subscription.plan.featuresList) {
subscription.plan.featuresList.forEach(feature => {
const existingFlatFeature = flatFeatures.find(flatFeatureItem => {
return (flatFeatureItem.name === feature.name);
});
if (existingFlatFeature) {
existingFlatFeature.expDate = (subscription.expDate > existingFlatFeature.expDate) ? subscription.expDate : existingFlatFeature.expDate;
} else {
flatFeatures.push({
name: feature.name,
expDate: subscription.expDate
});
}
});
}
});
1 Answer 1
A few thoughts;
- Most would write
const flatFeature = [];
to initialize an array in javascript, though what you have is certainly correct. - You should strive to de-nest your code. Having code unnecessarily placed in
if
orif-else
blocks increases the number of paths through your code, making it harder to follow and unit test. In this case, you could de-nest by changing your firstif
to something likeif (!subscription.plan || !subscription.plan.feature) return;
which clearly indicates that there is no work to do in this function unless those conditions are met. - You should strongly consider using an object to map your
flatFeature
names to the objects encountered with greatest expiration date. That eliminates the loopingfind()
operation to determine if an existingflatFeature
with that name has already been encountered, making this a constant time lookup. This may not be a big deal if there are 100 subscriptions being iterated, but what if there are 1 million? - Taking above approach of building a name-keyed object gives you flexibility to continue to work against those keyed feature names or you can simply use
Object.values(obj)
to get the array of feature objects. - Stylistically, your code looks somewhat "dense" and has long horizontal lines (especially your ternary) that make code harder to read and understand at a glance. Using javascript's destructuring can oftentimes help in keeping your code more terse and thus more easily read.
Putting it together, I might use something like:
const features = allActiveSubscriptions.reduce(
(obj, sub) => {
const { plan: { featuresList }, expDate } = sub;
if (!featuresList) return obj;
featuresList.forEach( ({ name }) => {
const existFeat = obj[name];
if (!existFeat) {
obj[name] = { name, expDate };
return;
}
if (expDate > existFeat.expDate) {
existFeat.expDate = expDate;
}
});
return obj;
},
{}
);
// if you want an array
const featArray = Object.values(features);
In terms of worst-case operational efficiency, this makes your function perform in O(m x n)
as opposed to O(m x n x p)
as it is currently (where m
is number of subscriptions, n
is maximum feature list size, and p
number of unique feature names).