Skip to content

fix: parse method mutating its input - #231

Merged
lance merged 1 commit into
cloudevents:masterfrom
matejvasek:fix-struct-parse
Jun 24, 2020
Merged

lance merged 1 commit into
cloudevents:masterfrom
matejvasek:fix-struct-parse

Conversation

@matejvasek

Copy link
Copy Markdown
Contributor

fixes: #230

@matejvasek

Copy link
Copy Markdown
Contributor Author

/cc @lholmquist @lance

};


const unprocessed = new Set(Object.keys(incoming));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is Set necessary here. Can't we assume that the keys in the object are unique already?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think they should be unique. The thing is: If I understand it correctly Object.keys(incoming) returns an array and deletion from an array is more complicated than just delete() which is offered by a Set object.

};


const unprocessed = new Set(Object.keys(incoming));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the most efficient would be using the spread operator like this.

Suggested change
const unprocessed = new Set(Object.keys(incoming));
const unprocessed = { ...incoming };

Signed-off-by: Matej Vasek <mvasek@redhat.com>
@matejvasek

Copy link
Copy Markdown
Contributor Author

@lance I force pushed it. There is now only one line changed: const incoming = { ...parser.parse(payload) };.

@lance lance left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Excellent - thank you!

@lance
lance merged commit 060b21b into cloudevents:master Jun 24, 2020
@lance lance mentioned this pull request Jul 7, 2020
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.

Receiver accept() method modifies its input parameter.

3 participants