stampit-org / stampit

OOP is better with stamps: Composable object factories.
https://stampit.js.org
MIT License
3.02k stars 102 forks source link

Need some guidance #331

Closed pendenaor closed 6 years ago

pendenaor commented 6 years ago

Post from https://stackoverflow.com/questions/48541276/stampit-js-beginner-needs-some-guidance has not received any answers, so i XPOST here. Please be mercyful 😄

I'm implementing service between a view and a Rest API.

Beside, i'm completly new to stamp programming and i'm in search of some advices about this sort of code:

import {compose, methods} from '@stamp/it'
import ArgOverProp from '@stamp/arg-over-prop'
import {template} from 'lodash'

const createLogger = name => (...args) => console.log('['+ name + ']', ...args)

const HasHttpClient = ArgOverProp.argOverProp('httpClient')
const HasApiVersion = ArgOverProp.argOverProp('apiVersion')
const HasUrl = ArgOverProp.argOverProp('url')

const UseRestApi = compose(HasHttpClient, HasApiVersion, HasUrl).init([
  function () {
    this.getUrl = template(this.url)
    this.useRestApiLog = createLogger('UseRestApi')
  }
]).methods({
  query: function query(method, {params, headers, body}) {
    const {apiVersion} = this
    const q = {
      baseURL: this.getUrl({apiVersion}),
      method,
      ...params != null && {params},
      ...headers != null && {headers},
      ...body != null && {body}
    }
    this.useRestApiLog('request config:', q)
    return q
  }
})

const WithGetOperation = compose(UseRestApi).init([
  function () {
    this.withGetOperationLog = createLogger('WithGetOperation')
  }
]).methods({
  'get': function get ({params}) {
    const q = this.query('get', {headers: {'Accept': 'application/json'}, params})
    this.withGetOperationLog('getting data')
    return this.httpClient(q)
  } 
})

const CustomerRestApi = compose(WithGetOperation).init([
  function () {
    this.customerRestApiLog = createLogger('CustomerRestApi')
  }
]).methods({
  all: function all() {
    this.customerRestApiLog('get all customers')
    return this.get({params: {page: 1, limit: 15}})
  }
})

const customerProvider = CustomerRestApi({
  url: 'http://sample.com/<%=apiVersion%>/customers',
  apiVersion: 'v1',
  httpClient: function(config) {
    return Promise.resolve({
      status: 200,
      config
    })
  }
})

const appLog = createLogger('APP')

customerProvider.all()
  .then(r => appLog('HTTP response code:', r.status))

Am i in the right directions?

Especially, the createLogger thing seems ugly!

How to inject a prefixed logger into each stamp ? How to extend that to warn, error, ... methods ?

koresar commented 6 years ago

Howdy!

Your logger looks just fine. 👍 It is not necessary to create every bit as a stamp. However, if you want to make the logger as a reusable stamp then you can do the same way as ArgOverProp is implemented.

Ruffly ArgOverProp is done this way:

const ArgOverProp = stampit.statics({
  argOverProp(...args) {
    return this.deepConf({ArgOverProp: [...args]});
  }
})
.init(function (options, {stamp}) {
  const {ArgOverProp} = stamp.compose.deepConfiguration;
  for (let assignableArgName of ArgOverProp) {
    this[assignableArgName] = options[assignableArgName];
  }
});

Your logger could look like this (not necessary exactly like this):

import {argOverProp} from '@stamp/arg-over-prop';

const Logger = stampit(
  argOverProp('prefix'), 
  {
    methods: {
      log(...args){ console.log(this.prefix, ...args); },
      error(...args){ console.error(this.prefix, ...args); },
      warn(...args){ console.warn(this.prefix, ...args); }
    }
  }
);

const HasLogger = stampit.statics({
  hasLogger(name) {
    return this.conf({HasLogger: {name}});
  }
})
.init(_, {stamp}) {
  const {HasLogger} = stamp.compose.configuration;
  if (HasLogger) {
    this.logger = Logger({prefix: HasLogger.name});
  }
});

And usage:

const CustomerRestApi = stampit(
  WithGetOperation, 
  HasLogger.hasLogger('CustomerRestApi'),
  {
    methods: {
      all() {
        this.logger.log('get all customers');
        return this.get({params: {page: 1, limit: 15}});
      }
  }
);

I always prefer readability. So, the code above, I hope, is readable to you and any stampit newbie. (Sorry for semicolons.)

PS: a tip. The stampit and the stampit.compose you imported above are the same exact function. :) See source code.

pendenaor commented 6 years ago

Nice design, thanks for sharing some stampit enlightment!

pendenaor commented 6 years ago

Sorry for re-opening this issue, are we agree that the HasLogger stamp cannot be composed with several stamps with their own prefixed log?

Below a sample to show what i mean :

import stampit from '@stamp/it';
import {argOverProp} from '@stamp/arg-over-prop';

const Logger = stampit(argOverProp('prefix'), {
    methods: {
      log(...args){ console.log(this.prefix, ...args); },
      error(...args){ console.error(this.prefix, ...args); },
      warn(...args){ console.warn(this.prefix, ...args); }
    }
  }
);

const HasLogger = stampit.statics({
  hasLogger(name) {
    return this.conf({HasLogger: {name}});
  }
})
.init(function (_, {stamp}) {
  const {HasLogger} = stamp.compose.configuration;
  if (HasLogger) {
    this.logger = Logger({prefix: HasLogger.name});
  }
});

const Stamp1 = stampit(HasLogger.hasLogger('Stamp1'), {
  methods: {
    fn1: function fn1(...args) {
      this.logger.log('fn1')
    }
  }
})

const Stamp2 = stampit(Stamp1, HasLogger.hasLogger('Stamp2'), {
  methods: {
    fn2: function fn1(...args) {
      this.logger.log('fn2')
      this.fn1()
    }
  }
})

const Stamp3 = stampit(Stamp2, HasLogger.hasLogger('Stamp3'), {
  methods: {
    fn3: function fn1(...args) {
      this.logger.log('fn3')
      this.fn2()
    }
  }
})

const s3 = Stamp3();
s3.fn3()
// Results
// Stamp3 fn3
// Stamp3 fn2 => Should be Stamp2 fn2
// Stamp3 fn1 => Should be Stamp1 fn1

Can i correct this behavior within HasLogger?

koresar commented 6 years ago

Sorry for the late reply. Vacation ongoing.

I'll rewrite your code using classes:

class HasLogger {
  constructor({ prefix }) {
    this.logger = Logger({ prefix });
  }
}

class Stamp1 extends Logger {
  static prefix = 'Stamp1'
  constructor({ prefix }) {
    super({ prefix: prefix || Stamp1.prefix });
  }
}

class Stamp2 extends Stamp1 {
  static prefix = 'Stamp2'
  constructor({ prefix }) {
    super({ prefix: prefix || Stamp2.prefix });
  }
}

class Stamp3 extends Stamp2 {
  static prefix = 'Stamp3'
  constructor({ prefix }) {
    super({ prefix: prefix || Stamp3.prefix });
  }
}

const s3 = new Stamp3();
s3.logger // WHAT YOU WILL HAVE HERE IS THE SAME AS IF THOSE WERE STAMPS

See code comment above.

You have the single object instance (s3) and want the this.logger to reference three different things (three loggers).

You better create a logger per each stamp. Something like that:

const Stamp1 = stampit({
  init() {
    this.loggerStamp1 = Logger({prefix: 'Stamp1'});
  },
  methods: {
    fn1(...args) {
      this.loggerStamp1.log('fn1')
    }
  }
})

Also, run this with you code: console.log(Stamp3.compose). It will print the metadata for the s3 object instance,

pendenaor commented 6 years ago

Ok, make me feel a little stupid 🙄 Thanks for your time.