From: "austin (Austin Ziegler)" Date: 2022-08-29T14:17:29+00:00 Subject: [ruby-core:109772] [Ruby master Feature#18959] Handle gracefully nil kwargs eg. **nil Issue #18959 has been updated by austin (Austin Ziegler). LevLukomskyi (Lev Lukomskyi) wrote in #note-25: > > You need to fail when someone passes you garbage (and `nil` is a garbage value in this case), or you need to ensure better default values (e.g., `{}`) at your call sites. > > When you pass `nil` you make less allocations, also you don't need to put `|| {}` at all calling sites. For an options which are just passed somewhere else inside the method it's a valid usage. You don���t need to put `|| {}` at all calling sites. The method and its documentation should be very clear that, if provided, the second parameter must be a Hash. That is, passing `nil` should be discouraged. Or you make the specific case that you want `nil` to *clear* the tracking info, in which case you probably shouldn���t be using `|| {}` or `**kwargs` at all. There may be value in `**nil => {}`, but I repeat that the examples you have provided are unconvincing, but are instead *likely* to have unintended side effects, especially as the examples you���ve given seem to be AR-ish in nature, and converting a hash to kwargs is not likely what you actually want with Active Record. > > I suspect that it can���t be optimized easily, because arbitrarily complex expressions can be specified after `if` or `unless`. > > I think it doesn't matter how complex condition there is, because the interpreter must fully evaluate it first anyway to check whether `if` is satisfied, then if the expression inside `if` is `{...}` and `**` operation is pending ��� just store entries in stack, not heap, and pass them to `**`. One more optimization could be to avoid creating an empty hash on `**nil`. Such an optimization might be possible (or more likely detecting `**nil` and doing nothing). > > I think that it���s *generally* a better change for the examples you have provided than `**nil` support. > > Actually that's not consistent that you are ok for `merge(nil)` but not ok for `add_email_to_list(tracking_info: nil)` which is kind of the same. Sure it is. The signature for `#merge` is `#merge(*hashes, &block)`, so the pseudo-code implementation could easily be: ```ruby class Hash def merge(*hashes, &block) hashes.each do |hash| next if hash.nil? # ��� do merge magic and conflict resolution if block has been given here end end end ``` Or, if you prefer ```ruby class Hash def merge(*hashes, &block) hashes.compact.each do |hash| # ��� do merge magic and conflict resolution if block has been given here end end end ``` Both of these options permit the "clever" code you���d eventually come to regret, but do so cleanly. > > > I���m still not fond of the pattern, and think that an explicit call to `Hash#delete_if` to remove defaulted values is clearer and more intentional, but whatever. > > I don't think that the way with `delete_if` is clearer since you mentally are doing two operations ��� first insert values into hash, and then remove them. In case of `**` you are doing only one operation ��� insert values. You���re looking at the concept of a `#default_options` method wrong. You���re not inserting values in the hash, you���re merging the *provided* values with defaults. Elixir has a function `Keyword.validate/2` that is called as `Keyword.validate(options, default_options)` and it ensures that the keyword list *only* has the keys present in `default_options` and that if those keys are missing, they���re set with the provided default value. This is more or less the same for mutable objects, and one additional step: having a way of removing values that should be omitted if not provided. I���ve done very similar things in JavaScript as well. The pattern is simple: `default_options.merge(*provided_options_list).delete_if { |h, k| omit_default?(h, k) }`. The use of `#merge` is intentional, because you don���t want to update the defaults (should they be stored in a constant), although `default_options.dup.update(*provided_option_list).delete_if {|h, k| omit_default?(h, k) }` would work as well. ---------------------------------------- Feature #18959: Handle gracefully nil kwargs eg. **nil https://bugs.ruby-lang.org/issues/18959#change-99009 * Author: LevLukomskyi (Lev Lukomskyi) * Status: Open * Priority: Normal ---------------------------------------- The issue: ```ruby def qwe(a: 1) end qwe(**nil) #=> fails with `no implicit conversion of nil into Hash (TypeError)` error { a:1, **nil } #=> fails with `no implicit conversion of nil into Hash (TypeError)` error ``` Reasoning: I found myself that I often want to insert a key/value to hash if a certain condition is met, and it's very convenient to do this inside hash syntax, eg.: ```ruby { some: 'value', **({ id: id } if id.present?), } ``` Such syntax is much more readable than: ```ruby h = { some: 'value' } h[:id] = id if id.present? h ``` Yes, it's possible to write like this: ```ruby { some: 'value', **(id.present? ? { id: id } : {}), } ``` but it adds unnecessary boilerplate noise. I enjoy writing something like this in ruby on rails: ```ruby content_tag :div, class: [*('is-hero' if hero), *('is-search-page' if search_page)].presence ``` If no conditions are met then the array is empty, then converted to nil by `presence`, and `class` attribute is not rendered if it's nil. It's short and so convenient! There should be a similar way for hashes! I found this issue here: https://bugs.ruby-lang.org/issues/8507 where "consistency" thing is discussed. While consistency is the right thing to do, I think the main point here is to have fun with programming, and being able to write stuff in a concise and readable way. Please, add this small feature to the language, that'd be so wonderful! ���� -- https://bugs.ruby-lang.org/ Unsubscribe: