Updating class method to use data from new weather API endpoint while keeping same return value object structure as with old endpoint

Question

I needed to update a class in Node.js that parses the response from a new weather API endpoint but still preserves the same object structure and key values from having been using the old weather API endpoint. The return value from the class' main method should be an array of eight objects, each representing data from a given day of the week, including hi and low temps, and a day and night object.

The data from the new API response is structured as follows with arrays of lengths 8 and 16:

const apiResponse = {
 dayOfWeek: ['Friday', 'Saturday', 'Sunday', 'Monday', 'Tuesday', 'Wednesday', 'Thursday', 'Friday'],
 calendarDayTemperatureMax: [57, 59, 53, 47, 42, 44, 42, 43],
 calendarDayTemperatureMin: [39, 40, 40, 37, 33, 31, 31, 30]
 validTimeLocal: ['2024-02-09T07:00:00-0500', '2024-02-10T07:00:00-0500', '2024-02-11T07:00:00-0500', '2024-02-12T07:00:00-0500', '2024-02-13T07:00:00-0500', '2024-02-14T07:00:00-0500', '2024-02-15T07:00:00-0500', '2024-02-16T07:00:00-0500'],
 daypart:[{
 dayOrNight: ['D', 'N', 'D', 'N', 'D', 'N', 'D', 'N', 'D', 'N', 'D', 'N', 'D', 'N', 'D', 'N'],
 daypartName: ['Today', 'Tonight', 'Tomorrow', 'Tomorrow night', 'Sunday', 'Sunday night', 'Monday', 'Monday night', 'Tuesday', 'Tuesday night', 'Wednesday', 'Wednesday night', 'Thursday', 'Thursday night', 'Friday', 'Friday night']
 temperature: [57, 40, 59, 45, 51, 37, 47, 35, 41, 31, 44, 31, 42, 30, 43, 29],
 iconCode: [32, 29, 26, 27, 26, 27, 39, 5, 16, 29, 30, 29, 30, 29, 30, 29]
 }]
}

The old weekly weather forecast class:

module.exports = class Weekly extends Feed {
 constructor () {
 super()
 this.desiredFields = ['daypart_name', 'temp', 'icon_code', 'fcst_valid_local']
 this.fieldMap = {
 'fcst_valid_local': 'date',
 'daypart_name': 'day_part'
 }
 }
 /**
 * Accepts Api results and request; formats feed for API output
 *
 * @param {Api} apiResponse
 * @param {Request} request
 */
 getDayPartFields (obj) {
 let fields = _.pick(obj, this.desiredFields)
 return Feed.mapFields(this.fieldMap, fields)
 }
 generateFeed (apiResponse, request) {
 let formattedResponse = apiResponse.forecasts.map( obj => {
 let { day, night, max_temp, min_temp} = obj
 let dayObject = this.getDayPartFields(day)
 let nightObject = this.getDayPartFields(night)
 return { hi: max_temp, lo: min_temp, day: dayObject, night: nightObject }
 })
 
 let feed = this.getBaseFeed(request, formattedResponse)
 this.setFeed(feed)
 }
}

My updated weekly weather forecast class looks like this:

module.exports = class Weekly extends Feed {
 constructor () {
 super()
 this.desiredFields = ['daypartName', 'temp', 'icon_code', 'validTimeLocal']
 this.fieldMap = {
 'validTimeLocal': 'date',
 'daypartName': 'day_part'
 }
 }
 /**
 * Accepts Api results and request; formats feed for API output
 *
 * @param {Api} apiResponse
 * @param {Request} request
 */
 generateFeed (apiResponse, request) {
 let formattedResponse = apiResponse.dayOfWeek.map((_, idx) => {
 const dayIndex = idx * 2;
 return {
 'hi': apiResponse.calendarDayTemperatureMax[idx],
 'lo': apiResponse.calendarDayTemperatureMin[idx],
 'day': {
 'daypart': apiResponse.daypart[0].daypartName[dayIndex],
 'temp': apiResponse.daypart[0].temperature[dayIndex],
 'icon_code': apiResponse.daypart[0].iconCode[dayIndex],
 'date': apiResponse.validTimeLocal[idx]
 },
 'night': {
 'daypart': apiResponse.daypart[0].daypartName[dayIndex + 1],
 'temp': apiResponse.daypart[0].temperature[dayIndex + 1],
 'icon_code': apiResponse.daypart[0].iconCode[dayIndex + 1],
 'date': apiResponse.validTimeLocal[idx]
 }
 };
 });
 
 let feed = this.getBaseFeed(request, formattedResponse)
 this.setFeed(feed)
 }
}

The properties in the constructor method were left over from the previous implementation, but I updated a couple of the values to correlate with the new API response. There were also a couple of helper methods with I deleted, each for populating the values in the day and night objects in the formattedResponse.

Would it be better to have a couple of helper methods here? Also, do I need to keep what's inside the constructor method? This was actually quite a bit of fun to solve, but I would welcome any suggestions for optimization, thanks!

Answer 1

This submission is about refactoring code so it will return a same-format response. Yet it did not include an automated test suite which shows Green bar on "old" code and again Green on this "new" code.

ctor

 daypartName: ['Today', 'Tonight', 'Tomorrow', 'Tomorrow night', 'Sunday', ... ]
 temperature: [57, ... ],
 iconCode: [32, ... ]
 ...
 this.desiredFields = ['daypartName', 'temp', 'icon_code', 'validTimeLocal']

I started to object to the names temp and icon_code, until I saw the fieldMap. It was initially unclear to me which were "old" and which were "new" field names. It would be helpful to put version numbers, dates, or vendor names on "old" and "new", for unambiguous reference in source comments. (Some European cities have had trouble with versioning their place names, leading to statements like "Let's meet for lunch near the Old New Bridge!")

It appears we're transitioning from a conventional camelCase endpoint to a snake_case endpoint. A comment in the source to that effect would be welcome. Then maintenance engineers will better understand why certain spelling variants were chosen, and how they should choose names when adding new features.

In particular, if the project architect has decided that code interacting with this should continue using camelCase, we should see a statement to that effect. Otherwise maintainers might figure "oh, we're transitioning everything to snake_case", and introduce undesired refactors.

do I need to keep what's inside the constructor method?

What you wrote looks good to me, modulo my remarks. Not sure what you would get rid of; it seems sensible from where I'm sitting.

day vs hour-within-day

 validTimeLocal: ['2024-02-09T07:00:00-0500',
 '2024-02-10T07:00:00-0500', ... ],
 ...
 'day': { ...
 'date': apiResponse.validTimeLocal[idx]
 },
 'night': { ...
 'date': apiResponse.validTimeLocal[idx]

I found those two lines of code confusing, in the sense that I simply don't know what date means here.

It appears to refer to a calendar day, e.g. 2024年02月09日. Yet 6:30am on the ninth would correspond to 2024年02月08日 ?!? And combining the 5 hour offset of EST (presumably from America/New_York) with 7 hours past midnight (start of day?) muddies the waters. It's unclear what edge cases a unit test author should look for, even for days that don't involve a Daylight Saving transition.

Summary: use the one-second resolution of HH:MM:SS when a detailed timestamp is intended, and prefer one-day granularity when an entire day is being modeled.

anonymous intermediate expression

 let feed = this.getBaseFeed(request, formattedResponse)
 this.setFeed(feed)

The code is perfectly eloquent here. No need to explain that a "feed" is what we got. Prefer a simple one-line assignment for this.

Would it be better to have a couple of helper methods here?

Nah. I'm usually pretty vocal about "too long!" or "DRY!", and I didn't notice any such issues here. If a unit test author comes along and wishes to expose formattedResponse, that will be straightforward, so we'll worry about crossing that bridge when we come to it.

• Aaron Goldsmith: Thanks so much for the feedback, I updated my question to include the older version of the class.