Since this is labelled TypeScript, I'm going to review it as such. First of all, you aren't using types, which defeats the purpose of using TypeScript. You should define what your objects look like
interface Datum { //not sure what to call it
continent: string,
country: string,
year: string,
value: number
}
I'm not a TypeScript expert or anything, but even as a beginner, I'd expect something at least like this. With this, we get that the year is apparently a string. Maybe it's not an issue but it's something to note. Before defining the type I just glanced at the object and assumed it was a number.
Which means that your array declaration should now look like this
const array : Array<Datum> = [
{continent: 'Africa', country:'Algeria', year:'2018', value:10},
{continent: 'Africa', country:'Algeria', year:'2017', value:15},
{continent: 'Africa', country:'Algeria', year:'2016', value:2},
{continent: 'Africa', country:'Egypt', year:'2018', value:20},
{continent: 'Africa', country:'Egypt', year:'2017', value:1},
{continent: 'Africa', country:'Egypt', year:'2016', value:20},
]
and the compiler would know what you are working with providing type safety for future operations.
Now, let's take a look at some other declarations
const yearsTemp : Array<string> = Array.from(new Set(dataset.map((datum: Datum) => datum.year)).values())
Assuming dataset has a content similar to array in the beginning, the types we annotate are Datum for each variable. This means that we are mapping each year value and get an array of string. However, calling .values() on the Set is superfluous - Array.from() works fine with just being passed a Set. So, you can drop that.
But what about the next line
const years: Array<string> = yearsTemp.map(year => year.toString())
So, we take an array of strings and produce an array of strings. That's fine normally, but to do that you call .toString() on each value. That's a pointless operation "some value".toString() is the same thing. It's also dangerous, as null.toString() would cause an error. For future reference, a way that wouldn't throw a null pointer exception is to pass the value through the String constructor function but don't use the keyword new.
let numericValue = 42;
let stringValue = String(numericValue);
console.log("typeof stringValue", typeof stringValue);
console.log('stringValue === "42"', stringValue === "42");
//this creates a new object string, not a string primitive
let dontDoThis = new String(numericValue);
console.log("typeof dontDoThis", typeof dontDoThis);
console.log('dontDoThis === "42"', dontDoThis === "42"); //not the same type
console.log('dontDoThis === new String("42")', dontDoThis === new String("42")); //not the same object
Passing the value null or undefined through String will not cause an error but it will produce the string "null" or "undefined" respectively. You can filter those out, if you don't want them
let objectsWithNumericValues = [{year: 2018}, {year: 2019}, {year: 2019}, {year: 2020}, {year: null}];
let extractedYears = objectsWithNumericValues.map(obj => obj.year);
let resultWithNull = Array.from(new Set(extractedYears))
.map(String); //<-- convert to string
let resultWithoutNull = Array.from(new Set(extractedYears))
.filter(Boolean) //<-- filter anything falsey
.map(String);
console.log(resultWithNull);
console.log(resultWithoutNull);
On a separate note, the above code shows how you can avoid temporary variables by chaining operations. It saves some typing and it's clearer by not having clutter like throw-away variable names. But it also depends on the coding style.
Anyway, back on the topic of getting the years and turning them into strings - just drop the line that does toString and rename const yearsTemp to const years
As for the rest it seems OK, as long as you add the types. It's not clear where the continent variable is coming from when you call filter(dataset, { continent: continent, year: year }) but I'd assume you have that in context. The other strange this is dataset[0].continent. I'm not sure if that's correct or not, as it would take the same continent for each new produced value and it's not necessarily going to be the correct one unless dataset[0].continent === continent. But that might be a typo when transcribing the code here. Perhaps the same is true for country: ''
The only change I'd make is separate the functionality instead of put all the processing into .map(). The chaining of array operation makes the separation of intentions clearer and if you need to extract functionality, it's cleaner.
years
.map(year => filter(dataset, { continent: continent, year: year }))
.map(filtered => {
const mean = filtered.reduce((acc, item) => acc + item.value, 0) / filtered.length
return {
continent: dataset[0].continent,
country: '',
year: year,
value: mean
}
})
this way if you decide that either or both functionalities are reusable, you can separate them and your logic turns into
years
.map(getResultsWithTheSameYear)
.map(calculateMean)
and you can use these functions elsewhere. Perhaps you will have some other functionality where you do
countries
.map(getResultsWithSameCountry)
.map(calculateMean)
filterdefined? \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Feb 4 '19 at 23:34filteris a lodash function (lodash.com/docs/4.17.11#filter). \$\endgroup\$ – sleepwalking Feb 5 '19 at 6:38