DEV Community

Discussion on: Refactoring: Functional Decorators

Collapse
 
dakujem profile image
Andrej Rypo • Edited

Hmm, reading this post, I failed to understand 2 things.

First, if funcIsExistingPerson function actually verified person existence (checked against DB or API or otherwise, that given id actually existed), how would hiding the logic from where the id comes be useful in any way? We provide the active route anyway, there is no point in calling

if (funcIsExistingPerson(this.ar))
Enter fullscreen mode Exit fullscreen mode

when we can write the following and obtain better transparency and code reusability of the funcIsExistingPerson function:

if ( funcIsExistingPerson( funcGetRouteId(this.ar) ) )
Enter fullscreen mode Exit fullscreen mode

Why? Well how about funcIsExistingPerson( my.friend.user.id )... You get the idea.

Second, since the funcIsExistingPerson does not check for person existence, how is better than simple? 🙃

if ( funcGetRouteId(this.ar) > 0 )
Enter fullscreen mode Exit fullscreen mode

In case the point is encapsulation of the id > 0 comparison, a much more reusable approach would simply be to create a generic function isIdValid and then

if ( isIdValid( funcGetRouteId(this.ar) ) )
Enter fullscreen mode Exit fullscreen mode

would:

  • hide the complexitiy in validating the id (encapsulate id > 0 check)
  • allow us to use it on other ids as well
  • make the code more readable at first glance (it is obvious that we are getting an id from a route and validating it even for someone who does not read the doc comment or source code of funcIsExistingPerson)
Collapse
 
jwp profile image
John Peters • Edited

For your first question, the example shown was to demonstrate the most simple example of a decorator possible to illustrate the concept. It's usefullness is questionable except where it's reused throughout the project. Which is the jest of this series.

The function turns out to be a business rule based on the person/Id route design, using numbers are not self evident whereas the function name is.

The other advantage is one time one place for that rule. Composing the function as you suggest is more visible but requires more knowledge of the API. Plus we have to change the implementation. One of the points in this series is to never change working code.
Knowledge of entire functional modules, becomes a large problem in using large libraries (Angular, React, Vue et. al.) because we don't really become that overtly knowledgeable until we've spent plenty of time with them.

Can we use this on other routes? No, this function is only for the person route which is why we put it in the person Typescript module.

Collapse
 
dakujem profile image
Andrej Rypo

I agree that the concept you describe is usable, but the example used is not.

The example mixes routing with business logic. I would never ever do that.

Furthermore, "never change working code" is just an excuse for not writing proper tests. A well tested aplication can be refactored without introducing bugs. The refactor I propose is "inversion of control", which is one of the most basic possible refactorings.

Yet, I agree that different working environments require different development practices. If "never changing working code" is one's dogma, it might make sense to encapsulate stuff into one-off functions.

Thread Thread
 
jwp profile image
John Peters • Edited

This article shows the huge advantage of this series. About halfway down we see how intellisense auto discovers all functions of a module. If each function is atomic and each module they are in, is properly named, there is no concerns regarding mixing of business logic with routing.

Our Person module/object is the aggregator of Person functions. This is Separation of Concerns in action. However as you stated before, we could easily composed functions explicitly, by using two modules: One for routing to get the ID, the other from Person to apply the rules. Or as suggested we use a decorator function with just one call. Because functions are fully atomic there is no mixing of Business logic, a function either has business logic or it doesn't. We may want to create a Business Logic module to keep them in the same place, but as far as function discovery they all become Viewable to intellisense with no distinction on where they came from other than the import statement.

Never change working code is "no excuse", it's the implementation of the Open/Closed principle. The principle is older than Javascript itself. Javascript breathes new life into the principle because functions are first class citizens. Open for extension is perfect when using the function. Closed for modification means all our previous tests should always work. We do however, need to create new tests for all new code.

One off functions in this example adhere to proper functional decorator techniques. Most newer programmers don't know about decorators. The results are always monolithic and repeated code patterns. This is always a total nightmare.

Thread Thread
 
dakujem profile image
Andrej Rypo

I believe I understand your point of view now 👍
I might have mistaken this example for a general one, it makes more sense in the context of Angular.

Thread Thread
 
jwp profile image
John Peters

This is not an Angular only concept it works in any Typescript or Javascript code. It's not limited by Angular in any way.