Re: [PATCH v2 1/2] t/helper/test-sha1: prepare for an unsafe mode

> So I think you wouldn't want to allocate an enum or a slot in the
> hash_algos array, because it is not really an independent algorithm.
> But I think it _could_ work as a separate struct that the caller derives
> from the main hash algorithm. For example, imagine that the
> git_hash_algo struct didn't have unsafe_init_fn, etc, but instead had:
> 
>   struct git_hash_algo *unsafe_implementation;
> 
> with a matching accessor like:
> 
>   struct git_hash_algo *unsafe_hash_algo(struct git_hash_algo *algo)
>   {
> 	/* if we have a faster "unsafe" implementation, use that */
> 	if (algo->unsafe_implementation)
> 		return algo->unsafe_implementation;
> 	/* otherwise just use the default one */
> 	return algo;
>   }
> 
> And then csum-file.c, rather than calling:
> 
>   the_hash_algo->unsafe_init_fn(...);
>   ...
>   the_hash_algo->unsafe_final_fn(...);
> 
> would do this:
> 
>   struct git_hash_algo *algo = unsafe_hash_algo(the_hash_algo);
>   algo->init_fn(...);
>   ...
>   algo->final_fn(...);
> 
> And likewise this test helper would have a single conditional at the
> start:
> 
>   if (unsafe)
> 	algo = unsafe_hash_algo(algo);
> 
> and the rest of the code would just use that.

> All that said, I do not think it buys us anything for "real" code. There
> the decision between safe/unsafe is in the context of how we are using
> it, and not based on some conditional. So while the code in this test
> helper is ugly, I don't think we'd ever write anything like that for
> real. So it may not be worth the effort to refactor into a more virtual
> object-oriented way.