Re: [PATCH v4 6/6] clean: teach clean -d to skip dirs containing ignored files

From: Junio C Hamano <gitster@pobox.com>
Date: May 22, 2017, 06:17 UTC
Message-ID: <xmqqk259o1o2.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAJZjrdX9BnuxY3tmpswG+yEdDm1+AR8rc5wKGZyVCMp-jP218A@mail.gmail.com>

Samuel Lijin <sxlijin@gmail.com> writes:

Show 11 quoted lines

>> By the way, instead of putting NULL, it may be easier to follow if
>> you used two pointers, src and dst, into dir.entries[], just like
>> you did in your latest version of [PATCH 4/6].  That way, you do not
>> have to change anything in the later loop that walks over elements
>> in the dir.entries[] array.  It would also help the logic easier to
>> follow if the above loop were its own helper function.
>
> Agreed on the helper function. On the src-dst thing: I considered it,
> but I figured another O(n) set of array moves was unnecessary. I guess
> this is one of those cases where premature optimization doesn't make
> sense?

I actually did not mean to give the variables more descriptive names and preserve the original 'main loop' (namely, not adding the "skip if NULL" which would never happen in normal case where "-d" is not used without "-x") as "optimization", whether it is premature or not. My suggestions were purely from "wouldn't the resulting code easier to follow and understand, leading to fewer bugs in the future?" point of view.

As I said, I am undecided if the result is easier to follow than your version ;-)