Skip to content

Conversation

@Renegatto
Copy link

No description provided.

'use strict';

const logable = (fields) =>
class Logable {
Copy link
Member

Choose a reason for hiding this comment

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

This shows class-returning function, rewriting will hide the essence of the example

},
});
const logable = fieldRules => data => {
var result = {}
Copy link
Member

Choose a reason for hiding this comment

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

Why var?

Suggested change
var result = {}
const result = {};

Comment on lines -40 to +34
const p1 = new Person({ name: 'Marcus Aurelius', born: 121 });
const p1 = Person({ name: 'Marcus Aurelius', born: 121 });
Copy link
Member

Choose a reason for hiding this comment

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

We use UpperCamel for classes and lowerCamep for functions, methods

});
const logable = fieldRules => data => {
var result = {}
Object.keys(fieldRules).forEach(key => {
Copy link
Member

Choose a reason for hiding this comment

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

keys/forEach will create array and make more function calls

})
result.toString = () =>
`Logable\t${
Object.keys(fieldRules).map(key => data[key] + '\t').join()
Copy link
Member

Choose a reason for hiding this comment

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

It is not ok to access data[key] from map, because data is out of mapping function scope, it will work but semantics does not fit

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.

2 participants