<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<meta http-equiv="Content-Style-Type" content="text/css" />
<meta name="generator" content="pandoc" />
<title></title>
<style type="text/css">
code{white-space: pre-wrap;}
span.smallcaps{font-variant: small-caps;}
span.underline{text-decoration: underline;}
div.column{display: inline-block; vertical-align: top; width: 50%;}
</style>
</head>
<body>
<p>Hi,</p>
<p>On 2018-07-20 11:24, Romain Vimont wrote:</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> Thank you for your review.</code></pre>
</blockquote>
<p>I am too tired to write code so no worries, in either case; you are welcome!</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> I assume you want to shrink *if* `array->i_capacity` is more than 1.5
times the current size, not if it is less than that.</code></pre>
</blockquote>
<pre><code> This is correct: if array->i_capacity is more than 1.5 times the current
size, then the if-condition is false, so we don't return immediately...
so we shrink.</code></pre>
</blockquote>
<p>This alone is proof that I should go and sleep, so I will do that straight after sending this email.</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> I probably should make this code more clear, it looks confusing.</code></pre>
</blockquote>
<p>Might just be me who has been awake for too long, though if you can make it more readable it is far from a bad thing - thanks.</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> Your tests should also assert on the return-code for functions which
may fail.</code></pre>
</blockquote>
<pre><code> I could, but I didn't do it because their failure would be triggered by
the following asserts anyway (e.g. if append() fails, count() will be
incorrect). And it would just add explicit assertions about
out-of-memory.</code></pre>
</blockquote>
<p>We have other tests using <code>asserts</code> in some places, and tests that even <code>assert</code> that memory-allocation succeeds. The reason is precisely as you pointed out: if <em>append</em> fails, <em>count</em> will be flagged as the culprit.</p>
<p>Even though you might know that the function will only fail due to a memory allocation, who knows what changes to the function will come in the future (if any)? Better safe than sorry, imo.</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> There must be a patch missing in this set, perhaps it is intentional,
but as we currently have no tests for arrays in `src/test/array.c` the
above is very odd.</code></pre>
</blockquote>
<pre><code> Yes, I noted in the cover-letter:</code></pre>
</blockquote>
<p>I should probably start reading things more carefully, instead of jumping straight to the implementation; sorry, my bad.</p>
</body>
</html>