Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Standardise argument conventions #7

Closed
kitsonk opened this issue Sep 19, 2015 · 8 comments
Closed

Standardise argument conventions #7

kitsonk opened this issue Sep 19, 2015 · 8 comments
Milestone

Comments

@kitsonk
Copy link
Member

kitsonk commented Sep 19, 2015

Based on the thoughts in dojo/core#28, I would like to propose the following...

That if an argument is not optional, in an API then it should be included in the signature of the function call. If the parameter is optional, then it should be included in in an argument called options which ideally will be defined using an exported interface and should be either a default or optional argument that is placed last.

Overloading and intersection types for clarity of the API should be used sparingly and when there is justification that it increases the utility or clarity of the API.

An example of some good APIs:

export interface FooOptions {
    bar?: string;
    baz?: number;
}

function fooA(location: string, options?: FooOptions): void {
    /* cool stuff */
}

function fooB(arg1: string|number, arg2: string, options?: FooOptions): void {
    /* cool stuff */
}

function fooC(options: FooOptions = { bar: 'bar' }): void {
    /* cool stuff */
}
@kitsonk kitsonk mentioned this issue Sep 19, 2015
@pottedmeat
Copy link

What if some combination of arguments is required and it would be difficult to define every possible signature?

@kitsonk
Copy link
Member Author

kitsonk commented Sep 19, 2015

Can you give an example of a rational usable API that has that sort of complex signature? I would seriously question its usability.

@pottedmeat
Copy link

A data store for example where there's an interface to make a query, but the implementations require different specific combinations.

@kitsonk
Copy link
Member Author

kitsonk commented Sep 19, 2015

Are you not talking about the type definition of an individual argument, versus the arity of a function? I guess in a way it is sort of related.

In order to define complex relationships like that in TypeScript, you would use and union of interfaces if it made sense for it to be more usable. If it is too complex then my recommendation would be to err on the side of optionality in the type definition and then throw during run time analysis. An example would be something like this:

interface StoreQuerySortFunction<T> {
    (a: T, b: T): number;
}

interface StoreQueryFilterFunction<T> {
    (item: T): boolean;
}

interface StoreQuerySort<T> {
    columns: string[],
    sort: string|StoreQuerySortFunction<T>
}

interface StoreQueryID {
    id: string;
}

interface StoreQueryFilter<T> extends StoreQuerySort<T> {
    filter: StoreQueryFilterFunction<T>;
}

function queryStore<T>(query: StoreQueryID<T>|StoreQuerySort<T>|StoreQueryFilter<T>): T[] {
    /* even more cooler stuff */
}

I still think there is great improvement to usability if we do the hard work of enumerating such things (or question if we have made an API too complex if it that hard to enumerate.

@pottedmeat
Copy link

Yeah, my assumption is that the set of use cases with complex dependencies would throw runtime errors. My point was more that "options" can appear in places where they're not all optional.

@kitsonk
Copy link
Member Author

kitsonk commented Sep 19, 2015

I would argue that is a design issue though. While you could say logically "when a is present, b must also be present" I would argue that it is better then to embed that further into the object literal, so it does become optional:

interface A {
  a: string;
  b: number;
}

interface FooOptions {
  c?: A;
}

function foo(options?: FooOptions): void {
    /* magical faerie dust */
}

Or otherwise if it is cleaner, then overload:

interface FooOptions {
    [key: string]: any;
}

function foo(options?: FooOptions): void;
function foo(a: string, b: number, options?: FooOptions): void;
function foo(...args: any[]): void {
  /* some sort of magic */
}

With the custom type guards in 1.6, you can narrow the arguments in the overloaded functions fairly easily.

@kitsonk kitsonk added this to the alpha.1 milestone Mar 11, 2016
@pdfernhout
Copy link
Contributor

tldr: I'd suggest using a configuration object for almost all API parameters when more than a couple well-defined ones are needed, almost always keeping to three or less total arguments in each API call -- and only using separate arguments for the first couple of API parameters when they clearly are different in category than the ones in the configuration object (like URL requested v. configuring how to do the request or handle errors).

=== More details

For human factors reasons, it is problematical to understand and use APIs in C-ish languages when there are more than three or four arguments. See for example:
http://stackoverflow.com/questions/174968/how-many-parameters-are-too-many

In Clean Code, Robert C. Martin devoted four pages to the subject. Here's the gist:
"The ideal number of arguments for a function is zero (niladic). Next comes one (monadic), followed closely by two (dyadic). Three arguments (triadic) should be avoided where possible. More than three (polyadic) requires very special justification -- and then shouldn't be used anyway."

One value of using objects with keys for APIs with many configuration options is you can easily tell what each argument was intended to mean. Support for keyword syntax is one reason Smalltalk code is so easy to read even when there are more than three arguments for a method.

Intuitively knowing what arguments might be truly "optional" when you go to make an API call would also require a deep understanding of the API and what it does internally. Since an application programmer probably won't know that, they probably still have to consult the documentation anyway to decide what arguments to put where. That's actually more work than just throwing everything into a configuration object.

Also, arguments might change in whether they are needed in future version. And let's say you need to add a new non-optional argument in the future. Do you add the new field after "options" to default it (an inconsistency versus the options arg always being at the end), or do you change the order of the parameters (and break backwards compatibility)?

Adding lots of specific options before a configuration option trades one kind of complexity (signalling what arguments are truly "optional", which can be done in other ways whether documentation or via TypeScript restrictions) for other kinds of complexity (limiting readability and limiting change). Which of these issues are more likely to be a major difficulty?

When referencing code example, it is also easier to just copy an entire configuration object and then use it elsewhere with changes than to copy part of the configuration and then copy non-optional arguments too.

An exception is perhaps when the options are qualitatively about a different thing. For example, in Dojo2 request, it may be OK to have an argument which is the URL for the resource you are getting, but then have a configuration object called "options" for how to get the URL and what to do if anything unusual happens. So, in that case, the two different arguments are arguably conceptually different in purpose. Below is another example, where "myData" is conceptually different from telling an algorithm how to process it.

interface ProcessConfig {
    tolerance_percent: number;
    distance_m?: number;
    weight_kg?: number;
}

let config: ProcessConfig = {
  tolerance_percent: 20,
  distance_m: 3.5,
  weight_kg: 9.0
};

process(myData, config);

Contrast that with this approach, which to me seems more confusing as the meaning of "20" is suddenly unclear (ignoring it might best be a named constant declared elsewhere):

interface ProcessConfig {
    distance_m?: number;
    weight_kg?: number;
}

let config: ProcessConfig = {
  distance_m: 3.5,
  weight_kg: 9.0
};

process(myData, 20, config);

For consistency, if I had to choose between naming some arguments and passing in others in an object versus passing everything as an object, I'd go with using just the object for configuration. TypeScript makes that choice more feasible because you could specify what fields of the object are required and which ones are optional (as in the above example).

As above from "Clean Code", a different design rule to consider might be you can put up to two always-required arguments before options -- and that is the maximum. And, if there are more than two non-optional arguments, all arguments should be passed in as a structure. If you have three or less options and absolutely no possibility of ever having more, than you can skip using the configuration object and just have arguments (but it is risky, as you will likely end up with a fourth eventually). As a special case, if options are in wildly different categories, it might be worth considering using multiple arguments each with a different configuration object -- again though, limiting the total number of arguments to three, and otherwise going with one top-level configuration object that has multiple nested configuration objects.

The above is just an idea. As with the Stackoverflow article, there are various reasons to have stylistic disagreements over all this. "Code Complete" suggests up to seven -- possibly based on George Miller's "The magic number seven" essay. But even he later said seven comes out of a combination of both short-term memory (three or four) and rehearsal memory (another three or so). Thus three (or at most four) is a safer limit ergonomically for "chunks" you expect people to think about at once.

As the accepted answer as SO says: "I hate making hard and fast rules like this because the answer changes not only depending on the size and scope of your project, but I think it changes even down to the module level. Depending on what your method is doing, or what the class is supposed to represent, it's quite possible that 2 arguments is too many and is a symptom of too much coupling."

@kitsonk kitsonk modified the milestones: 2016.04, alpha.1 Apr 8, 2016
@kitsonk kitsonk modified the milestones: 2016.05, 2016.04 May 3, 2016
@kitsonk kitsonk modified the milestones: 2016.06, 2016.05 Jun 7, 2016
@kitsonk kitsonk modified the milestones: 2016.06, 2016.07 Jul 4, 2016
@kitsonk kitsonk modified the milestones: 2016.07, 2016.08 Aug 1, 2016
@kitsonk kitsonk modified the milestones: 2016.09, 2016.08 Oct 4, 2016
@kitsonk
Copy link
Member Author

kitsonk commented Oct 4, 2016

These concepts have been largely adopted, closing and we can re-open if there are other challenges.

@kitsonk kitsonk closed this as completed Oct 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants