Is this procedural programming?

It all started with "I'll make a couple of helper functions" and the next thing I know there were procedures everywhere.

This is a "speed typer" from the vanillawebprojects repository.

This post was created with the "share to DEV" button on codepen. It is a very convenient feature.

EDIT:
Here is a less messy version: link

Comments

[Jeremy Forsythe]

Your functions (e.g. new_time()) are modifying state outside themselves. You should be passing in the state and returning a new state. You effectively have a global variable.

[Heiker]

"Global" in the game scope, but not really global global.

You should be passing in the state and returning a new state.

I could, but I don't know exactly where should I store the new state.

[Jeremy Forsythe]

That's why I said effectively global. It's in a closure but multiple parties are changing it, so it suffers from most of the problems that a true global does.

You could do something like

state = getStateWithNewTime(state);

And replace the state object with a new object when you need to mutate it. The outer scope still holds the state in a variable but the functions don't ever access it directly. Instead they take in the current state as a parameter and return a new state (important to still not mutate the parameter but instead return a new state object.. the spread operator makes this dead simple).

[Heiker]

I understand that immutability has benefits and overall is a good practice, but in here I don't see any reason to force it. With getStateWithNewTime I still have a function that exists just to modify the time, still use the state in the closure and now I have to remember to replace the old state in every single place.

For a second I want it to use a central store and put the mutations in there, but with just three variables in the state I could not justify to myself doing that.

[Jeremy Forsythe]

Here's an example. It's not perfect, but I think it's much easier to follow and roughly the same number of lines of code.

The main differences are:

1. The view (DOM updates) are separated from the logic. Instead of having a utility function to update each property of the state and updating the view in the same function, this updates the whole state in one step and updates the view in a separate step. The timer is also separated from the state.
2. The state object is only mutated in two places - nextWord() and in the interval callback. This makes it much easier to track changes to the state object. In your code I counted at least 7 different places where the state object gets mutated
3. Removed the Num class as it doesn't add any benefit - there are already numbers and you can already add, subtract, and retrieve them

You can extract everything above the game() function into separate files and easily write unit tests for the functions because they're pure. Testing the View class would require mocks/stubs because DOM interaction is an integration point. Everything left inside the game() function is an integration between the state/logic, view, and timer so you wouldn't write unit tests for that code.

I wouldn't call this "forcing" immutability. It is easier to grok, easier to follow, easier to debug, and easier to test. It follows separation of concerns and limits shared state to only the points of integration.

[Heiker]

I wouldn't call this "forcing" immutability. It is easier to grok, easier to follow, easier to debug, and easier to test. It follows separation of concerns and limits shared state to only the points of integration.

You did refactor the entire thing. If the point you were trying to make was about immutability, I didn't get it. Using tecniques to make your data immutable in a messy code, still leaves you with a messy code.

If this was about separation of concerns and code organization, you should have said it from the beginning, I would have agreed with you.

[Jeremy Forsythe]

The getStateWithNewWord() function gets your new state without mutating it and without calling 3 separate functions to update the state. It is much easier to see how the state is changing this way. That was the main point. Instead of 7 state mutations, it mostly happens in one place.

You're right, simply refactoring the state mutations into a single function left the code messy, hence the larger refactor. Separation of concerns goes hand in hand with techniques like immutability because it keeps the state change where it belongs.

I didn't mean to come off as negative in any way so apologies if it seems that way.

Your original question was whether your code is "procedural". I'd say it would be considered "spaghetti" due to the lack of separation of concerns.

I assumed the question was due to the negative connotation around procedural code. My refactor is procedural but I don't necessarily consider that a bad thing. It still follows good programming practice and gets the job done.