How to handle default values for dependency injection? about light-service HOT 9 CLOSED

Said that I'm just a user, and probably I'm raising a more philosophical than a practical argument: given your example I'd go for a PORO. I can't see any advantage here using LightService. You're not using the "functional" approach of using the context as single source of truth, neither the flow control facilities to control success/failure from the outside of the service object.

If I'd have to force your example to be a use case for LS (but it's probably a useless excercise)

class Format
  extend ::LightService::Action

  expects :items
  promises :formatted_items

  executed do |ctx|
    ctx.formatted_items = begin
      ctx.items.map { |item| ItemPresenter.present(item) }
    rescue
      ctx.fail!('Cannot decorate the collection')
    end
  end
end

class Export
  extend ::LightService::Action

  expects :formatted_items
  promises :csv

  executed do |ctx|
    ctx.csv = begin
      ItemToCsv.build_file(formatted_items)
    rescue
      ctx.fail!('Error during export')
    end
  end
end

class ExportItemsOrganizer
  extend ::LightService::Organizer

  def self.call(items)
    with(items: items).reduce(
      Fomrat,
      Export
    )
  end
end

items = ItemRepository.by_date(today_date, tomorrow_date)
result = ExportItemsOrganizer.call(items)

return DoSomethingWith(result.csv) if result.succes?

EmailTheErrorToMe(result.message)

This would be worth the effort because you can clean the outer code, but still retain control on what's happening and taking decisions about it.

But if I'd have only the need to group up calls to already defined and standalone services, then I'd use a PORO.

I also always take into account one aspect of LS design: all is forced to happen in class context, not instance. This forces you to not store state inside actions classes, because the only state should be the context passed down into and transformed by each action.

Moreover: all your services are already defined and standalone, which usually is enough to unit test them one-by-one. Testing an action or an organizer should (IMO IMO IMO) always happen in integration: call the action/organizer feeding it with initial context, checking for failure/success.

Oh...I ended not replying to your very question. Sorry about that. Just chatting. :)

Have a nice day/evening

from light-service.

Here I was referring to the fact that to get a list of all the dependencies an action is using you need to read its executed block, while with a standard PORO constructor, you'd have all the deps listed in the initialize method, and then used in the run method.

Now I got it. Fortunately is still valid the reasoning to move them from initializer to the "manifest" (Injector in my example, but whatever will be into your codebase).

Whatever your path will be, good luck!

from light-service.

I came to this repo to post something very similar. It would be great if LS supported default values for expects params.

Our use case is that we have a few very important actions that have a very common default case, but need to behave slightly differently under certain circumstances. It would be great if we could set a default flag treated as a first class expected context value, and then override that flag in the few places it is required, instead of changing all the code that uses that action to know about the flag.

I envision the API looking something like this, perhaps?

class AddsThreeNumbers
  extend ::LightService::Action

  expects :first_number
  expects :second_number, default: 10
  expects :third_number,  default: ->(ctx) { ctx.second_number.even? ? 7 : 13 }
 
  promises :result

  executed do |ctx|
    val = ctx.first_number + \
    ctx.second_number + \ # Assuming it was not explicitly passed into the context, is 10
    ctx.third_number # Evaluated dynamically from previous ctx declarations, is 13

    ctx.result = val
  end
end

Calling this action in these combinations yields:

AddsThreeNumbers.execute(first_number: 1, second_number: 1, third_number: 1).result # => 3 - all expects are static
AddsThreeNumbers.execute(first_number: 1, second_number: 1).result # => 15 - third_num default is calced dynamically
AddsThreeNumbers.execute(first_number: 1, second_number: 2).result # => 9 - third_num default is calced dynamically
AddsThreeNumbers.execute(first_number: 1, third_number: 1).result # => 12 - second_num uses static default
AddsThreeNumbers.execute(first_number: 1).result # => 18 - second_num uses static default, third_num is evaled 
AddsThreeNumbers.execute() # => ExpectedKeys exception

@adomokos - I'd be really keen to take a stab at implementing this. If I was able to pull it off, would you want LS to have this functionality? From my standpoint, it is backward compatible, and doesn't add extra complexity unless you specifically wish to use it.

from light-service.

@metalelf0 inspired by your problem I tried to figure out if a full fledged dependencies injector would work well together with a LS organizer.

I'd like to leave here this example if anyone else would be interested in the approach: https://gist.github.com/pioneerskies/a2ddc9c504af10a99e7a2b3cee8bc951

from light-service.

Hi Alessandro, thanks for taking the time to build this code. I think this strategy could be a viable solution to implement dependency injection. I only see the following possible improvements:

• having a per-organizer dependency container vs a per-action dependency container might end up with a lot of dependencies, and it wouldn't be clear which dependency is used in which action (unless you look into each action code). It would be simpler to implement and maintain, though, so depending on how complex the organized code is, YMMV.
• every action expects a deps object, but it's not clear which dependencies an action is going to use until you read the implementation. In traditional OO code, I'd make dependencies explicit in the constructor (like @increment = deps.increment), but this would defy the functional philosophy of LS (everything is in the context, no state should exist in actions).

What do you think about this?

from light-service.

Hi Alessandro, thanks for taking the time to build this code.

Hey it was really faster than going for English ;)

• having a per-organizer dependency container vs a per-action dependency container might end up with a lot of dependencies, and it wouldn't be clear which dependency is used in which action (unless you look into each action code). It would be simpler to implement and maintain, though, so depending on how complex the organized code is, YMMV.

• If with "container" you're referencing what I've called Injector, yes: you could have a lot of dependencies. The fact you've a object with the single responsibility to list dependencies moves the object itself from "verbose" to "declarative" IMO
• it's also true that you should read the implementation. But is that really different having in your action

foo = MyExternalDep.call

versus

foo = context.deps.my_external_dep.call

?

Watching the code from the Organizer perspective, instead, should not be a matter of knowing the implementation of single actions. I'd say I'd be happy to not knowing anything about actions until I'll have to modify one.

• «so depending on how complex the organized code is» I'm totally with you about this. It's a fact.

• every action expects a deps object, but it's not clear which dependencies an action is going to use until you read the implementation. In traditional OO code, I'd make dependencies explicit in the constructor (like @increment = deps.increment), but this would defy the functional philosophy of LS (everything is in the context, no state should exist in actions).

• «In traditional OO code, I'd make dependencies explicit in the constructor» Yep. The injector library itself is designed to use it like

include Import['foo_dep']

mixing it directly into your instance, but having stateless actions we cannot. Thus I thought about the injector parameter to be passed into organizer's context. By the way we've to observe that we can easily inspect context.deps object to read all the deps or we can read the Injector class wich actually is a "manifest".
Using this approach you have to like the fact that it's not an action's responsibility to know about exactly what dependency it needs, but just the interface to use on it. Opinionated stuff. Like all of our code, I mean :P

• «but it's not clear which dependencies an action is going to use until you read the implementation» I understand the same from a variable name (deps.my_external_service) than from a class name (MyExternalService). Maybe it's really subjective :)

from light-service.

Hey :) Code always speaks better, haha!

I'll give your suggestion a try and see where it leads. Actually, thinking of it like a manifest puts it under a different perspective, that I like much more.

About the last point:

but it's not clear which dependencies an action is going to use until you read the implementation» I understand the same from a variable name (deps.my_external_service) than from a class name (MyExternalService). Maybe it's really subjective :)

Here I was referring to the fact that to get a list of all the dependencies an action is using you need to read its executed block, while with a standard PORO constructor, you'd have all the deps listed in the initialize method, and then used in the run method.

from light-service.

Thanks! If you happen to be in Milan someday and wanna have a chat, drop me a line :) Have a great day!

from light-service.

@adomokos - I'd be really keen to take a stab at implementing this. If I was able to pull it off, would you want LS to have this functionality? From my standpoint, it is backward compatible, and doesn't add extra complexity unless you specifically wish to use it.

Yes, please!!!

from light-service.