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

Unflat json #62

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Unflat json #62

wants to merge 2 commits into from

Conversation

Parul-rathva
Copy link

Description

  • Added Unflat function.
  • Added Unit test case for unflattening a simple object

Related Issue

Add unflat Function to Restore Flattened JSON Structures #48

Checklist

  • I have tested these changes locally.
  • I have updated relevant documentation.
  • I have added appropriate comments and/or commit messages.
  • My code follows the project's coding conventions and style guidelines.

@Parul-rathva Parul-rathva marked this pull request as ready for review June 8, 2024 17:57
@@ -81,3 +81,37 @@ describe("flat functon", () => {
expect(flat(data)).toEqual(expected);
});
});


describe("unflat functon", () => {
Copy link
Owner

Choose a reason for hiding this comment

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

typo: unflat function

});

// Test case for unflattening a simple object
it("Unflattens a simple object", () => {
Copy link
Owner

Choose a reason for hiding this comment

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

lint: remove extra tab

} else {
acc[k] = acc[k] || {};
}
return acc[k];
Copy link
Owner

Choose a reason for hiding this comment

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

What this return doing?

Comment on lines +23 to +29
keys.reduce((acc, k, i) => {
if (i === keys.length - 1) {
acc[k] = data[key];
} else {
acc[k] = acc[k] || {};
}
return acc[k];
Copy link
Owner

Choose a reason for hiding this comment

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

Please use meaningful argument/variables names.

const unflat = (data, delimiter = ".") => {
return Object.keys(data).reduce((unflattenedObject, key) => {
const keys = key.split(delimiter);
keys.reduce((acc, k, i) => {
Copy link
Owner

Choose a reason for hiding this comment

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

This reduce function is being used but its output isn't directly utilized. This approach is valid, but it can be a bit confusing. Using a simple forEach loop might make the intent clearer.

Comment on lines +24 to +28
if (i === keys.length - 1) {
acc[k] = data[key];
} else {
acc[k] = acc[k] || {};
}
Copy link
Owner

Choose a reason for hiding this comment

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

Please add a comment here to clarify the code's purpose.

const expected = {};
expect(unflat(data)).toEqual(expected);
});
})
Copy link
Owner

Choose a reason for hiding this comment

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

Please add a few more tests that cover scenarios with more deeply nested objects, and also add module tests for cjs and esm.

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.

None yet

2 participants