-
Notifications
You must be signed in to change notification settings - Fork 13
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 test for subnetwork reducers #42
Comments
Take this issue |
@nolleto I think that is no need to refactor with const pathSubnetwork = path(['payload', 'state', 'network', 'subnetworks']);
const defaultToEmptyArray = defaultTo([]);
const onLoadNetwork = pipe(pathSubnetwork, defaultToEmptyArray);
const onRemoveSuperNode = (action) => filter(propEq('id', action.payload.id))
export default (state = [], action) => {
switch (action.type) {
case NEW_NETWORK:
return defaultToEmptyArray();
case ADD_SUPER_NODE:
return [
...state,
formatNetwork(action.payload.state),
];
case REMOVE_SUPER_NODE:
return onRemoveSuperNode(action)(state)
case LOAD_NETWORK: {
return onLoadNetwork(action)
}
default:
return state;
}
}; |
Hello @ufocoder Yeah, the refactor of this small reducer has not a great impact on readability, for example. This is just a refactor to keep it more functional and I believe just const removeSuperNode = (state, action) =>
state.filter(n => n.id !== action.payload.id);
const getPayloadId = path(['payload', 'id']);
const removeSuperNodeWithRamda = (state, action) =>
reject(propEq('id', getPayloadId(action)), state);
removeSuperNode([], {}) // throws an error
removeSuperNodeWithRamda([], {}) // returns a new state: [] Another thing, I believe the refactor will be something like: const addNetworkProps = ({ network, nodes, positions }) => ({
...network,
nodes,
positions,
color: getRandomColor(),
})
const formatNetwork = ifElse(hasNodesAndPositions, addNetworkProps, prop('network'))
const getPayloadState = path(['payload', 'state'])
const getPayloadSubnetworks = path(['payload', 'state', 'network', 'subnetworks'])
const getPayloadId = path(['payload', 'id'])
const getNetworkFromPayloadState = pipe(getPayloadState, formatNetwork)
const getPayloadSubnetworksOrDefault = pipe(getPayloadSubnetworks, defaultTo([]))
export default (state = [], action = {}) => {
switch (action.type) {
case NEW_NETWORK:
return [];
case ADD_SUPER_NODE:
return append(getNetworkFromPayloadState(action), state);
case REMOVE_SUPER_NODE:
return reject(propEq('id', getPayloadId(action)), state);
case LOAD_NETWORK:
return getPayloadSubnetworksOrDefault(action);
default:
return state;
}
}; 😄 |
This issue can ben two PRs but, do the tests first to make sure nothing breaks.
Also, you can use ramda to refactor:
Other details about this reducer:
colorsNol
that should be something likegetRandomColor
. I believe you can extract this to a new utils like/src/utils/colors
or use a package for it. Just to be clear: a super-node is a sub-network that will receive a random color. Yes, I'm awere that super-node is a really bad name but I will change it in another PR.if (nodes && positions) {
. This was made because we can receive two different networks: the current format and the old.networks.zip
In this
.zip
you can find the two networks. You can move theses networks to a__fixtures__
folder and then use it in your tests.first-version-network.json
- the first/legacy versionsimple-network.json
- the current versionWell, thats it 😄
The text was updated successfully, but these errors were encountered: