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

add task solution #2617

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

Conversation

MateuszSlezak
Copy link

No description provided.

Copy link

@Meg-Sowka Meg-Sowka left a comment

Choose a reason for hiding this comment

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

Looks like it works nicely, BUT you should have taken a look at the checklist before approaching it ;)

Copy link

@choeqq choeqq left a comment

Choose a reason for hiding this comment

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

Almost there 🚀

Comment on lines 11 to 13
const ADD_PROPERTIES = order.type === 'addProperties';
const REMOVE_PROPERTIES = order.type === 'removeProperties';
const CLEAR_ALL = order.type === 'clear';
Copy link

Choose a reason for hiding this comment

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

those should be constants outside of function - not depending on order.type

Comment on lines 15 to 35
if (ADD_PROPERTIES) {
Object.assign(state, order.extraData);
stateHistory.push(Object.assign({}, state));
}

if (REMOVE_PROPERTIES) {
for (const remove of order.keysToRemove) {
delete state[remove];
}

stateHistory.push(Object.assign({}, state));
}

if (CLEAR_ALL) {
for (const key in state) {
delete state[key];
}

stateHistory.push(Object.assign({}, state));
}
}
Copy link

Choose a reason for hiding this comment

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

according to checklist you should use switch statement

@@ -5,7 +5,36 @@
* @param {Object[]} actions
*/
function transformState(state, actions) {
// write code here
const stateHistory = [];
Copy link

Choose a reason for hiding this comment

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

you weren't prompted to track state history, but it's good idea 😄

Copy link
Author

Choose a reason for hiding this comment

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

so i keep this ;p

Copy link

@choeqq choeqq left a comment

Choose a reason for hiding this comment

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

Waiting for any updates @MateuszSlezak 👀

Copy link

@choeqq choeqq left a comment

Choose a reason for hiding this comment

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

Good job ✅

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.

3 participants