https://redmine.ruby-lang.org/https://redmine.ruby-lang.org/favicon.ico?17113305112020-03-04T23:42:29ZRuby Issue Tracking SystemRuby master - Bug #16660: Struct#deconstruct_keys inconsistent behavior https://redmine.ruby-lang.org/issues/16660?journal_id=844882020-03-04T23:42:29Zpalkan (Vladimir Dementyev)dementiev.vm@gmail.com
<ul></ul><p>(For some reason cannot edit the original post).</p>
<blockquote>
<p>Prior to that change an empty hash was returned in both cases.</p>
</blockquote>
<p>I think, the less confusing behavior would be to ignore the unknown keys without "failing fast" (i.e., returning an empty Hash if key is unknown).</p>
<p>That is:</p>
<pre><code class="ruby syntaxhl" data-language="ruby"><span class="n">s</span><span class="p">.</span><span class="nf">deconstruct_keys</span><span class="p">([</span><span class="ss">:a</span><span class="p">,</span> <span class="ss">:c</span><span class="p">])</span>
<span class="c1">#=> {a: 1}</span>
<span class="n">s</span><span class="p">.</span><span class="nf">deconstruct_keys</span><span class="p">([</span><span class="ss">:a</span><span class="p">,</span> <span class="ss">:b</span><span class="p">,</span> <span class="ss">:c</span><span class="p">])</span>
<span class="c1">#=> {a: 1, b: 2}</span>
</code></pre>
<p>IMO, that better reflects the pattern matching: if some key is present in one pattern (one key set), it must be present in another pattern with the same key.</p>
<p>I guess, that have been done for optimization (fail-fast if more keys then we have) but I believe that such optimization should be done within the pattern matching implementation, not a particular class API. The proposed behaviour would be even easier to optimize (e.g., we can re-use the deconstruction hash for keys subsets or cache the result of the key presence check).</p> Ruby master - Bug #16660: Struct#deconstruct_keys inconsistent behavior https://redmine.ruby-lang.org/issues/16660?journal_id=849732020-04-09T02:30:14Zmame (Yusuke Endoh)mame@ruby-lang.org
<ul><li><strong>Assignee</strong> set to <i>ktsj (Kazuki Tsujimoto)</i></li></ul> Ruby master - Bug #16660: Struct#deconstruct_keys inconsistent behavior https://redmine.ruby-lang.org/issues/16660?journal_id=849862020-04-10T05:31:44Zmatz (Yukihiro Matsumoto)matz@ruby.or.jp
<ul></ul><p>I agree that it's inconsistent and (a little bit) confusing. But it's not a bug.<br>
I wish <a class="user active user-mention" href="https://redmine.ruby-lang.org/users/3007">@ktsj (Kazuki Tsujimoto)</a> to revisit this issue, but I honor his decision.</p>
<p>Matz.</p> Ruby master - Bug #16660: Struct#deconstruct_keys inconsistent behavior https://redmine.ruby-lang.org/issues/16660?journal_id=850472020-04-11T11:34:27Zktsj (Kazuki Tsujimoto)kazuki@callcc.net
<ul><li><strong>Status</strong> changed from <i>Open</i> to <i>Rejected</i></li></ul><p>My stance is "if we really should care about consistency over performance, we should remove the <code>keys</code> argument itself from the specification.<br>
Otherwise, we should do a thorough optimization using the <code>keys</code> argument(*)".</p>
<p>Since <code>deconstruct_keys</code> is supposed to be used implicitly in the context of pattern matching rather than explicitly by the user,<br>
I don't think this inconsistency will actually cause confusion for the user.</p>
<p>(*) This means that the return value of <code>deconstruct_keys</code> is implementation-dependent.</p>
<hr>
<blockquote>
<p>I believe that such optimization should be done within the pattern matching implementation, not a particular class API.</p>
</blockquote>
<p>Let's compare <code>Struct#deconstruct_keys</code> to <code>Hash#deconstruct_keys</code>.</p>
<pre><code class="ruby syntaxhl" data-language="ruby"><span class="c1"># Struct#deconstruct_keys(palkan's version)</span>
<span class="n">s</span><span class="p">.</span><span class="nf">deconstruct_keys</span><span class="p">([</span><span class="ss">:a</span><span class="p">,</span> <span class="ss">:c</span><span class="p">])</span>
<span class="c1">#=> {a: 1}</span>
<span class="c1"># Hash#deconstruct_keys(2.7's version)</span>
<span class="n">h</span> <span class="o">=</span> <span class="p">{</span><span class="ss">a: </span><span class="mi">1</span><span class="p">,</span> <span class="ss">b: </span><span class="mi">2</span><span class="p">}</span>
<span class="n">h</span><span class="p">.</span><span class="nf">deconstruct_keys</span><span class="p">([</span><span class="ss">:a</span><span class="p">,</span> <span class="ss">:c</span><span class="p">])</span>
<span class="c1">#=> {a: 1, b: 2}</span>
</code></pre>
<p>Should <code>h.deconstruct_keys([:a, :c])</code> also return <code>{a: 1}</code>?<br>
I don't think so.</p>
<p>The optimal return value for each class is different.<br>
Therefore, we should implement optimized <code>#deconstruct_keys</code> in each class.</p>
<blockquote>
<p>we can re-use the deconstruction hash for keys subsets or cache the result of the key presence check</p>
</blockquote>
<p>The return value can still be cached in the current specification.</p>
<p>For example:</p>
<pre><code class="ruby syntaxhl" data-language="ruby"><span class="k">case</span> <span class="n">val</span>
<span class="k">in</span> <span class="p">{</span><span class="n">a</span><span class="p">:,</span> <span class="n">b</span><span class="p">:,</span> <span class="n">c</span><span class="p">:}</span>
<span class="k">in</span> <span class="p">{</span><span class="n">a</span><span class="p">:,</span> <span class="n">b</span><span class="p">:}</span>
<span class="k">end</span>
</code></pre>
<p>We can compile this code as following pseudo code:</p>
<pre><code class="ruby syntaxhl" data-language="ruby"><span class="k">case</span> <span class="n">val</span>
<span class="k">in</span> <span class="p">{</span><span class="n">a</span><span class="p">:,</span> <span class="n">b</span><span class="p">:}</span> <span class="o">&&</span> <span class="p">{</span><span class="n">c</span><span class="p">:}</span>
<span class="k">in</span> <span class="p">{</span><span class="n">a</span><span class="p">:,</span> <span class="n">b</span><span class="p">:}</span>
<span class="k">end</span>
</code></pre>