Re: [PATCH 08/17] revert: Separate cmdline parsing from functional code

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jul 12, 2011, 18:20 UTC
Message-ID: <20110712182046.GD14120@elie>
In-Reply-To: <1310396048-24925-9-git-send-email-artagnon@gmail.com>

Hi,

Ramkumar Ramachandra wrote:

> Currently, revert_or_cherry_pick does too many things including
> argument parsing and setting up to pick the commits; this doesn't make
> a good API.

On the contrary, generally speaking single-call APIs are often very pleasant to work with. Consider the difference between

	struct compressor c;
	memset(&c, 0, sizeof(c));
	init_compressor(&c);
	set_compressor_opts(c, COMPRESSOR_OPT_FAST);
	set_compressor_input("foo.c");
	set_compressor_output("foo.c.compressed");
	while (compressor_pump() != COMPRESSOR_DONE)
		; /* just looping */
	fini_compressor(&c);

and

	compress_file("foo.c.compressed", "foo.c", COMPRESSOR_OPT_FAST);

The latter is more pleasant, right?

So this is probably not about the function doing too many things but something else.

By the way, the API still has the problem described in [1]: it could not be used from another translation unit, since one of the parameters (the "me" variable) is passed through a global instead of on the stack or in registers.

If the goal is a pleasant API, a good way to explain it is sometimes

	Currently you have to do:

...

	After this change, you can write:

...

	Isn't that nice?

[1] http://thread.gmane.org/gmane.comp.version-control.git/176647/focus=176668