Skip to content
This repository has been archived by the owner on Oct 13, 2024. It is now read-only.

Input validation refactor #10

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

Conversation

SheriefBadran
Copy link
Contributor

No description provided.

…ode to be more functional and reactive, alot of text input is now handled as a streams. The goal is to make the code easier to maintain, move around and change. Hopefully some code gets more testable even if I did not succeed with tests so far.
@SheriefBadran
Copy link
Contributor Author

Tests fail since Rxjs implementation, spent some time trying to figure out how to fix the tests but so far without success. I will make further effort later on.

@SheriefBadran SheriefBadran force-pushed the inputValidationRefactor branch from 4a50e82 to 910a252 Compare January 11, 2016 22:22
…ction is central in this work. Functions execuded with respect to expectedInput are now in a separate file, they are also pure and we know that all data (pretty much) they are working with is passed into them. The switch statement is also in a function that is close to pure, the only reference from outside is the constants.
@SheriefBadran SheriefBadran force-pushed the inputValidationRefactor branch from 910a252 to 7b9ce39 Compare January 11, 2016 22:55
@krawaller
Copy link
Contributor

@SheriefBadran, could you add a high-level description of the refactor in a comment here?

@SheriefBadran
Copy link
Contributor Author

Of course @krawaller. The main focus have been to refactor the main export function in the inputvalidation.js file. That function had

  1. A block of if else statements handling input commands from the user.
  2. After that a switch statement to handle different cases of expectedInput.

The first part is now done with an Rx observable broadcasting input from the user. The observeCommand function is central for how the first part work, and is implemented in the inputValidationHelpers file. This function is preconfigured (curried) with a command-type constant, a predicate function for wanted filtering and a pre configured dispatch function. The configured observe function is finally called with an observable, and an observable/stream for a certain command is returned. All streams are merged into one dispatchable stream that is subscribed upon. The final dispatch then talkes place when subscribing. Also pay attention to the generateDispatchable function in the inputValidationHelpers file, it returns an object with dispatchable functions, the object properties are computed to avoid string dependency problems. In general, strings are now replaced with constants pretty much everywhere within the scope of this PR. Moreover constants are now in a freezed object.

The second part was a switch statement with cases calling most of the functions in the same file (inputvalidation.js). Each function is passed all the data it needs. Those function need a lot of different parameters, hence they are curried, imported in inputvalidation.js preconfigured with required input and then passed into handleExpectedInput, a curried function in expectedInputHandler.js. In this function the switch statement is executed with the passed in preconfigured functions.

The purpose is to make function more independent and pure. Also generally divide code into smaller peaces that is easy to pass around.

Maybe not so high-level, but hope it is clear enough.

@SheriefBadran
Copy link
Contributor Author

ok? :)

@SheriefBadran SheriefBadran force-pushed the inputValidationRefactor branch from 401f1a3 to bb2dfce Compare January 12, 2016 19:27
@SheriefBadran
Copy link
Contributor Author

Hmm, tests pass locally. I don't get this.

@SheriefBadran
Copy link
Contributor Author

Heheh, hard life, 0,02% from success ;)

@SheriefBadran SheriefBadran force-pushed the inputValidationRefactor branch from e9aecd2 to cf472dc Compare January 12, 2016 21:23
@@ -0,0 +1,4 @@
import R from 'ramda';

export const firstWordOf = (string) => R.compose(R.head, R.split(' '))(string);
Copy link
Contributor

Choose a reason for hiding this comment

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

It is so pretty!! <3

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants