LF-4625 Readability Refactors on Ensemble controller code #3663
Conversation
… for readability Remove reduce in favour of more readable array methods; encapsulate all three Ensemble methods into one function; define a constant for 'Ensemble Scientific'
…adability Remove unused returned values from registerOrganizationWebhook; un-nest code run when function does not return early; add comments to make more explicit how bulkSensorClaim response is defined from Ensemble API response
kathyavini
requested review from
antsgar,
SayakaOno and
Duncan-Brain
and removed request for
a team
There was a problem hiding this comment.
I am good with this since if you think it will help, we are deleting it all mostly is my understanding though.
reduce() I believe is much faster for large volumes, and small reduce functions are fairly readable (I like (acc, cv) over (previous, current) from other author though ;) But filter + map is also readable if this is for looking back.
| await FarmExternalIntegrationsModel.updateWebhookId(farmId, response.data.id); | ||
| return { ...response.data, status: response.status }; | ||
| }; | ||
| await ensembleAPICall(accessToken, axiosObject, onError, onResponse); |
There was a problem hiding this comment.
While unused the return statement gave the option to see result of webhook call. Wondering if error message throwing change with return placement too.
|
@SayakaOno I am more interested in readability before premature optimization more just a personal note on small reduce functions and finding them fairly readable. I also like filter + map. Not authoritative reading I did lately for |
Description
(As mentioned in tech daily Jan 23rd) These readability refactors on the Ensemble portion of the controller code were done as part of the ticket scope of the original controller ticket. The changes are limited to using a constant where there was a hardcoded string, refactoring out a few
.reduce()functions for readability, and clarifying a few return statements that were set up misleadingly. A lot of the Ensemble code (probably whole add flow, maybe webhook function as well) is going to be removed in our "remove old sensors code" task, but I wanted to merge the refactors because:Removing CSV is not going to be part of this ticket; that should happen in the cleanup/removal task.
Jira link: https://lite-farm.atlassian.net/browse/LF-4625
Type of change
How Has This Been Tested?
Checklist: