Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

create function transposeObjectArray #31

Merged
merged 6 commits into from
Jan 14, 2025
Merged

Conversation

abrantesarthur
Copy link
Member

Related Issues

  • [none]

Security Implications

[none]

System Availability

[none]

@abrantesarthur abrantesarthur requested review from anotherminh and a team January 14, 2025 18:15
* // rest: [{age: 25, city: 'NY'}, {age: 30, city: 'LA'}]
* // }
*/
export const partitionObjectArray = <T extends object, K extends keyof T>(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this naming is not super clear, I thought this was splitting by condition like lodash's partition. Maybe we could use something like invertObjectArray or transformObjectsToPropertyArray or pivotObjectArray (like excel pivot) or transposeObjectArray

Copy link
Member Author

@abrantesarthur abrantesarthur Jan 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially thought of extractProperties, as in "we are extracting these properties from the array". Wdyt?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm that doesn't feel clear enough still. Maybe extractPropertyArrays? but that also doesn't feel satisfying

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually think transposeObjectArray is a great name! Will update it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool, thanks!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is Claude's argument for transpose 😆 :

Yes, transposeObjectArray is actually a very fitting name! Here's why:

  • In mathematics, transposition involves converting rows into columns (or vice versa), which is conceptually similar to what this function does:
  // From:
   [
     { id: 1, name: 'John' },
     { id: 2, name: 'Jane' }
   ]
   
   // To:
   {
     id: [1, 2],         // column of ids
     name: ['John', 'Jane'], // column of names
     rest: [{}, {}]      // remaining columns
   }
  • It's more precise than "partition" because we're not just splitting the data, we're reorganizing it into a different structure

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah that was my thought process too for that name

@abrantesarthur abrantesarthur changed the title create function partitionObjectArray create function transposeObjectArray Jan 14, 2025
Comment on lines +73 to +80
const restObject = {} as Omit<T, K>;
Object.entries(item).forEach(([key, value]) => {
if (!properties.includes(key as K)) {
(restObject as any)[key] = value;
}
});

result.rest = [...(acc.rest || []), restObject];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe this should be an opt-in options prop

...
options: { includeOtherProperties?: boolean },

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea! i will push this in a follow-up PR so I can merge it already.

@abrantesarthur abrantesarthur merged commit 9d00d53 into main Jan 14, 2025
11 checks passed
@abrantesarthur abrantesarthur deleted the extractProperties branch January 14, 2025 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants