These are chat archives for dry-rb/chat

19th
Jan 2016
Tim Riley
@timriley
Jan 19 2016 01:55
Looks like dry-auto_inject doesn’t play nicely with manual arguments that you want to pass to an initializer too?
Piotr Solnica
@solnic
Jan 19 2016 09:18
@timriley manual arguments? It works with positioned args only for now. I'll make it more flexible soon though
Tim Riley
@timriley
Jan 19 2016 09:18
Hi!
I meant more something like this:
class MyClass
  include MyApp::Import(“some_dep”, “another_dep”)

  def initialize(some_config, *deps)
    @some_config = some_config
    super(*deps)
  end
end
I forked dry-auto_inject and had a go at trying to make it work, but didn’t get there.
I was just trying to see if I could get something working within the existing structure.
I think I got stuck at the fact that auto-inject’s generated initialize can’t really know what args the user has provided in their own custom initialize.
Then I thought “maybe prepend can help here…” and then I realised that in this case I could make things work well enough if I just moved my custom “config” arg into #call for now :)
Piotr Solnica
@solnic
Jan 19 2016 09:26
In general auto inject should be used for composition only. Unspecified number of args would be a design smell
Tim Riley
@timriley
Jan 19 2016 09:27
So my situation was “I want this class to have some initializer-provided config as well as injected collaborators.”
I thought that having static config passed via the initializer was reasonable.
Piotr Solnica
@solnic
Jan 19 2016 09:47
Can't you inject the config via import?
Tim Riley
@timriley
Jan 19 2016 09:48
In this case it was a string :)
Piotr Solnica
@solnic
Jan 19 2016 16:28
@AMHOL are you going to release new dry-configurable? I’d like to upgrade dependency in dry-validation (releasing a new version tomorrow)
Andy Holland
@AMHOL
Jan 19 2016 16:31
@solnic was going to do it after work tonight
Unless you want to do it?
Think you have access?
Piotr Solnica
@solnic
Jan 19 2016 16:32
no time today
so please go ahead :)
I need to wrap up dry-v update and then do client work
Andy Holland
@AMHOL
Jan 19 2016 16:33
Cool :)
Andy Holland
@AMHOL
Jan 19 2016 16:49
Released it
Luca Guidi
@jodosha
Jan 19 2016 16:50
@AMHOL :clap:
Andy Holland
@AMHOL
Jan 19 2016 16:50
:)
Luca Guidi
@jodosha
Jan 19 2016 16:51
@AMHOL @timriley @solnic starting from tomorrow, I'll have a closer look at DRY and try to integrate your awesome code with Lotus :sparkles:
Andy Holland
@AMHOL
Jan 19 2016 16:57
@jodosha that's cool :) Feel free to give me a shout if you need anything
Piotr Solnica
@solnic
Jan 19 2016 16:57
@jodosha please do keep me posted and ask any questions you may have, all of the dry stuff is shaping up and we’re polishing the apis/fixing bugs actively
Luca Guidi
@jodosha
Jan 19 2016 16:57
@AMHOL I'll start looking at Data and Validation in combination with ROM
Piotr Solnica
@solnic
Jan 19 2016 16:58
lotus + rom = :dancers:
Luca Guidi
@jodosha
Jan 19 2016 16:58
@solnic Sure, it's just that I didn't had the chance to start yet
Piotr Solnica
@solnic
Jan 19 2016 16:58
I’ll be pushing a new release of dry-v with a huge improvement in high-lvl rule composition
so stay tuned
Luca Guidi
@jodosha
Jan 19 2016 16:58
:heart_eyes:
Fran Worley
@fran-worley
Jan 19 2016 16:59
I can vouch for the update. Its FAB :clap:
Piotr Solnica
@solnic
Jan 19 2016 17:00
@fran-worley btw, I’m wrapping up support for rule groups as we speak, so rule(email: :presence) { .. } will work :)
then in errors you can specify email: { presence: “some msg” }
Fran Worley
@fran-worley
Jan 19 2016 17:02
@solnic amazing will that also work for your confirmation shortcut?
Piotr Solnica
@solnic
Jan 19 2016 17:03
@fran-worley I…dunno :joy: we’ll see
Fran Worley
@fran-worley
Jan 19 2016 17:05
@solnic I like your attitude! :heart_eyes_cat:
Piotr Solnica
@solnic
Jan 19 2016 17:12
hah :) just trying to stay positive
Piotr Solnica
@solnic
Jan 19 2016 17:38
result.reduce { |a, e|
  e.merge(a) { |_, l, r| l.is_a?(Hash) ? l.merge(r) : l + r }
}
I just wrote this, and I’m not ashamed of it :joy:
dryrb/dry-validation@81ace07 <= @fran-worley could you see if this works for you?
Fran Worley
@fran-worley
Jan 19 2016 17:43
@solnic Looking now...
Andy Holland
@AMHOL
Jan 19 2016 17:44
@solnic could transproc be used there?
Piotr Solnica
@solnic
Jan 19 2016 17:45
@AMHOL nah man, makes no sense to pull in transproc just to replace a single method
esp that performance is a concern here, so optimized method is better
Andy Holland
@AMHOL
Jan 19 2016 17:46
:+1:
Interdependencies FTW tho, get those numbers on the rubygems downloads :joy:
Piotr Solnica
@solnic
Jan 19 2016 17:53
hahaha
Fran Worley
@fran-worley
Jan 19 2016 17:59
@solnic It nearly works. Is there a reason why it's coming through in a double array?
{:email=>[["must be provided if login access granted"]]}
that causes the ["must be provided if login access granted"] to be outputted to the user
Piotr Solnica
@solnic
Jan 19 2016 18:02
oh uh, hold on
@fran-worley done, please git pull :)
as in, bundle update dry-v
btw confirmation stuff more work, I won’t make it today, please stay on master until tomorrow, I’ll finish this up and push 0.6.0 (API change is needed hence the minor ver bump)
in general, I’m gonna need a lot of feedback from a bunch of folks re error msgs structure, ie I’m undecided whether providing original input in the messages is needed
I’m also undecided whether to provide a richer object or stick to the plain hash like we have now
things like that must be sorted out prior 1.0 :)
Fran Worley
@fran-worley
Jan 19 2016 18:08
@solnic Your latest commit works so thanks. No worries about the confirmation stuff I've worked around it for now. I get where you are going. probably best to see how others use it. No point in adding rich objects if its not needed!
simplicity is part of why I love this gem!
Piotr Solnica
@solnic
Jan 19 2016 18:09
yeah I’m all for avoiding complex objects when hash is sufficient
ActiveModel::Validations
                       172.000  i/100ms
      dry-validation   899.000  i/100ms
-------------------------------------------------
ActiveModel::Validations
                          1.723k (± 6.6%) i/s -      8.600k
      dry-validation      9.092k (± 4.8%) i/s -     45.849k

Comparison:
      dry-validation:     9091.8 i/s
ActiveModel::Validations:     1722.6 i/s - 5.28x slower
still #winning!
Luca Guidi
@jodosha
Jan 19 2016 18:10
:heart:
Piotr Solnica
@solnic
Jan 19 2016 18:10
the last big thing is error message optimization via message prioritization, this one’s gonna be fun to implement
if things go well we should get another speed boost :D
Fran Worley
@fran-worley
Jan 19 2016 18:11
Yup. Its awesome and another bonus to AMV is that it can validate dates to out of the box!
No more hideous custom validator classes!
Piotr Solnica
@solnic
Jan 19 2016 18:12
that was one of many reasons why I started working on this :) I’ve got tons of personal pet peeves wrt validation that I’m trying to solve here
ie lack of type-safety was always killing me
“oh, damn, I can’t validate this w/o checking the type” <= said solnic to himself 1M times
btw I’d like to encourage people to experiment with custom macros based on existing APIs…we’ve got one for now, the confirmation one, but we should be able to easily find many common patterns of usage and encapsulate that in simple one-liner-macros
for rspec lovers: assuming(:login).is(true).check_presence_of(:email) :joy:
I mean…not kidding, this is totally possible, not sure if makes sense though :D
Luca Guidi
@jodosha
Jan 19 2016 18:17
Impressive :+1:
Fran Worley
@fran-worley
Jan 19 2016 18:17
I'm keeping a gist with examples of validations I'm using.
I'll convert it from reform to pure dry-v and post it
Piotr Solnica
@solnic
Jan 19 2016 18:17
@fran-worley that’s awesome, we could compile new docs from this kind of notes, thank you so much for your help
Fran Worley
@fran-worley
Jan 19 2016 18:19
No worries. Your idea on custom macros would enable us to make an easy transition from AMV without sacrifising the beauty of dry-v
Tim Riley
@timriley
Jan 19 2016 21:00
One thing that’s nice about keeping the original input in the messages structure is that when you have an each validation, it’s possible to work out which input had which set of errors.
At least that’s how I’m using it.
Hannes Nevalainen
@kwando
Jan 19 2016 21:01
^^ what he said
Piotr Solnica
@solnic
Jan 19 2016 21:12
right, now I remember why I did it lol
Tim Riley
@timriley
Jan 19 2016 21:12
:)
Fran Worley
@fran-worley
Jan 19 2016 21:29

Can you think of a cleaner way of defining this:

key(:login, &:bool?)
rule(password: :presence) do
  value(:login).true?.then(rule(:password).filled?)
end
rule(password_confirmation: :presence) do
  value(:login).true?.then(rule(:password_confirmation).filled?)
end
rule(password_confirmation: :confirms_password) do
  value(:login).true?.then(rule(:password_confirmation, eql?: [:password, :password_confirmation]))
end

I need to define each rule separately to return the correct error message otherwise I'd flatten the lot anyway.

I thought this might be cool:

key(:login, &:bool?)
value(:login).true?.then do
  key(:password, &:filled?)
  key(:password_confirmation, &:filled?)
  rule(:password_confirmation, eql?: [:password, :password_confirmation])
end

But then figured that this work probably work as well:

key(:login, &:bool?)
if value(:login).true?
  key(:password, &:filled?)
  key(:password_confirmation, &:filled?)
  rule(:password_confirmation, eql?: [:password, :password_confirmation])
end
Or even
key(:login, &:bool?)
confirmation(:password) if value(:login).true?
Piotr Solnica
@solnic
Jan 19 2016 21:49
@fran-worley I’ll make it possible to do rule(:require_password) { value(:login).true?.then(confirmation(:password)) }
or sth similar
need to make sure it plays nicely with error msgs
Fran Worley
@fran-worley
Jan 19 2016 21:50
Yeah that's what I'm playing with now. Its easy to correctly identify the validity, alot harder to give a meaningful message!
I'm just playing about with ideas really my app has alot of conditionals going on with validations.
Piotr Solnica
@solnic
Jan 19 2016 21:51
that’s one of major concerns in dry-v, so I’ll be working on this prior 1.0
the goal is to always generate the absolute minimum amount of messages
Fran Worley
@fran-worley
Jan 19 2016 21:53
True, but just telling the user that a field isn't valid isn't always helpful. They like to know why. Was it because it wasn't the correct format, or it was empty when it shouldn't have been etc.
Piotr Solnica
@solnic
Jan 19 2016 21:53
yeah, that’s why you need a way to provide custom messages
and sane defaults should be provided
I’m hoping that high-level rules will be helpful here, because you can name them in a sensible way and provide customized msgs that will serve as a good feedback for the end-user
Fran Worley
@fran-worley
Jan 19 2016 21:57
Yup makes sense, so rather than breaking each stage down (e.g. a message for filled, a message for format) just validate the lot in one rule and provide one message to the user: "You must provide a value in the format XYZ" rather than AMVs way of triggering the presence message "can't be blank", then once that's sorted triggering the format message. I've been thinking about this all wrong
Piotr Solnica
@solnic
Jan 19 2016 21:58
yeah, pretty much
it’s not gonna be trivial to achieve though, I expect a couple of iterations until it’s good-to-go
Fran Worley
@fran-worley
Jan 19 2016 22:00
Yeah no worries, if you ever want to me to test some commits through my app let me know. I have a dry-v branch I'm playing with :wink2:
Piotr Solnica
@solnic
Jan 19 2016 22:00
@fran-worley thanks, will do :)
Tim Riley
@timriley
Jan 19 2016 22:09
@fran-worley it’s awesome seeing you dive into this :)
Piotr Solnica
@solnic
Jan 19 2016 22:09
:+1: no lib will ever grow without early adoption and feedback, this is super helpful