47

In Clean Code the author gives an example of

assertExpectedEqualsActual(expected, actual)

vs

assertEquals(expected, actual)

with the former claimed to be more clear because it removes the need to remember where the arguments go and the potential misuse that comes from that. Yet, I've never seen an example of the former naming scheme in any code and see the latter all the time. Why don't coders adopt the former if it is, as the author asserts, clearer than the latter?

closed as primarily opinion-based by gnat, amon, Greg Burghardt, Thomas Junk, John R. Strohm Jul 2 '18 at 15:18

Many good questions generate some degree of opinion based on expert experience, but answers to this question will tend to be almost entirely based on opinions, rather than facts, references, or specific expertise. If this question can be reworded to fit the rules in the help center, please edit the question.

  • 9
    I think this is great question for a discussion. But not something that can be answered with objective answer. So this question might be closed as opinion-based. – Euphoric Jun 30 '18 at 12:46
  • 54
    Many people would argue against the first naming scheme because it's excessively verbose, far beyond the point where it would aid clarity. Especially for assertEquals(), that method is used hundreds of time in a code base so it can be expected that readers familiarize themselves with the convention once. Different frameworks have different conventions (e.g. (actual, expected) or an agnostic (left, right)`), but in my experience that's at most a minor source of confusion. – amon Jun 30 '18 at 13:03
  • 5
    Because gain is so small, compared to its benefits, that any sane one would probably walk away. If you want a more fluent approach you should try assert(a).toEqual(b) (even if IMO it's still needlessly verbose) where you may chain few related assertions. – Adriano Repetti Jun 30 '18 at 13:43
  • 18
    How do we know that actual and expected are values? Surely it should be assertExpectedValueEqualsActualValue? But wait, how do we remember whether it uses == or .equals or Object.equals? Should it be assertExpectedValueEqualsMethodReturnsTrueWithActualValueParameter? – immibis Jul 1 '18 at 10:28
  • 6
    Given that, for this particular method, the order of the two arguments doesn't matter, it seems like a poor example to choose to espouse the benefits of this naming scheme. – Steven Rands Jul 2 '18 at 8:46
65

Because it is more to type and more to read

The simplest reason is that people like to type less, and encoding that information means more typing. When reading it, every time I have to read the whole thing even if I am familiar with what the order of the arguments should be. Even if not familiar with the order of arguments...

Many developers use IDEs

IDEs often provide a mechanism for seeing the documentation for a given method by hovering or via a keyboard shortcut. Because of this, the names of the parameters are always at hand.

Encoding the arguments introduces duplication and coupling

The names of the parameters should already document what they are. By writing the names out in the method name, we are duplicating that information in the method signature as well. We also create a coupling between the method name and the parameters. Say expected and actual are confusing to our users. Going from assertEquals(expected, actual) to assertEquals(planned, real) doesn't require changing the client code using the function. Going from assertExpectedEqualsActual(expected, actual) to assertPlannedEqualsReal(planned, real) means a breaking change to the API. Or we don't change the method name, which quickly becomes confusing.

Use types instead of ambiguous arguments

The real issue is that we have ambiguous arguments that are easily switched because they are the same type. We can instead use our type system and our compiler to enforce the correct order:

class Expected<T> {
    private T value;
    Expected(T value) { this.value = value; }
    static Expected<T> is(T value) { return new Expected<T>(value); }
}

class Actual<T> {
    private T value;
    Actual(T value) { this.value = value; }
    static Actual<T> is(T value) { return new Actual<T>(value); }
}

static assertEquals(Expected<T> expected, Actual<T> actual) { /* ... */ }

// How it is used
assertEquals(Expected.is(10), Actual.is(x));

This can then be enforced at the compiler level and guarantees that you cannot get them backwards. Approaching from a different angle, this is essentially what the Hamcrest library does for tests.

  • 5
    Well, if you use an IDE you have the parameter names in the balloon help; if you don't use one, remembering the function name is equivalent to remembering the arguments, so nothing is gained either way. – Peter A. Schneider Jun 30 '18 at 21:44
  • 29
    If you object to assertExpectedEqualsActual "because it is more to type and more to read", then how can you advocate assertEquals(Expected.is(10), Actual.is(x))? – ruakh Jul 1 '18 at 3:14
  • 9
    @ruakh it’s not comparable. assertExpectedEqualsActual still requires the programmer to care to specify the arguments in the right order. The assertEquals(Expected<T> expected, Actual<T> actual) signature uses the compiler to enforce the correct usage, which is an entirely different approach. You can optimize this approach for brevity, e.g. expect(10).equalsActual(x), but that was not the question… – Holger Jul 1 '18 at 11:42
  • 6
    Also, in this particular case (==), the order of arguments is actually irrelevant to the final value. The order matters only for a side effect (reporting the failure). When ordering matters, it may make (marginally) more sense. For example strcpy(dest, src). – Kristian H Jul 1 '18 at 12:27
  • 1
    Can't agree more, especially with the part on duplication & coupling... If every time a function parameter changes it's name, the function name would also have to change, you'd have to go track all the usages of that function and change them as well... That would make a boatload of breaking changes for me, my team, and all the others using our code as a dependency... – mrsmn Jul 2 '18 at 10:16
20

You ask about a long standing debate in programming. How much verbosity is good? As a general answer, developers have found that the extra verbosity naming the arguments is not worth it.

Verbosity does not always mean more clarity. Consider

copyFromSourceStreamToDestinationStreamWithoutBlocking(fileStreamFromChoosePreferredOutputDialog, heuristicallyDecidedSourceFileHandle)

versus

copy(output, source)

Both contain the same bug, but did we actually make it any easier to find that bug? As a general rule, the easiest thing to debug is when everything is maximally terse, except the few things which have the bug, and those are verbose enough to tell you what went wrong.

There's a long history of adding verbosity. For example, there's the generally-unpopular "Hungarian notation" which gave us wonderful names like lpszName. That has generally fallen by the wayside in the general programmer populace. However, adding characters to member variable names (like mName or m_Name or name_) continues to have popularity in some circles. Others dropped that entirely. I happen to work on a physics simulation codebase whose coding style documents require that any function which returns a vector must specify the vector's frame in the function call (getPositionECEF).

You might be interested in some of the languages made popular by Apple. Objective-C includes the argument names as part of the function signature (The function [atm withdrawFundsFrom: account usingPin: userProvidedPin] is written in the documentation as withdrawFundsFrom:usingPin:. That's the name of the function). Swift made a similar set of decisions, requiring you to put the argument names in the function calls (greet(person: "Bob", day: "Tuesday")).

  • 13
    All other points aside, that would be far easier to read if copyFromSourceStreamToDestinationStreamWithoutBlocking(fileStreamFromChoosePreferredOutputDialog, heuristicallyDecidedSourceFileHandle) were written copy_from_source_stream_to_destination_stream_without_blocking(file_stream_from_choose_preferred_output_dialog, heuristically_decided_source_file_handle). See how much easier that was?! That’s because it’s too easy to miss small changes midway through that humungousunbrokenwordsalad, and it takes longer to figure out where the word boundaries are. Smashing confuses. – tchrist Jul 1 '18 at 6:26
  • 1
    The obj-C syntax withdrawFundsFrom: account usingPin: userProvidedPin is actually borrowed from SmallTalk. – joH1 Jul 1 '18 at 17:11
  • 14
    @tchrist be careful with being certain you are right on topics involving holy wars. The other side is not always wrong. – Cort Ammon Jul 1 '18 at 22:12
  • 3
    @tchrist Addingunderscoresnakesthingseasiertoreadnotharderasyousee is manipulating the argument. The answer here used capitalization, which you're omitting. AddingCapitalizationMakesThingsEasyEnoughToReadAsYouCanSeeHere. Secondly, 9 times out of 10, a name should never grow beyond [verb][adjective][noun] (where each block is optional), a format which is well readable using simple capitalization: ReadSimpleName – Flater Jul 2 '18 at 7:55
  • 5
    @tchrist - the science of your study (free full text link) simply shows that programmers trained to use underscore style are faster at reading underscore style than camel case. The data also shows that the difference is smaller for more experienced subjects (and most of the subjects being students suggest that even those were likely not especially experienced). This doesn't mean that programmers who have spent more time using camel case will also give the same result. – Jules Jul 2 '18 at 8:04
8

The author of "Clean Code" points out a legitimate problem, but his suggested solution is rather inelegant. There are usually better ways to improve unclear method names.

He is right that assertEquals (from xUnit style unit test libraries) does not make it clear which argument is the expected and which is the actual. This have also bit me! Many unit test libraries have noted the issue and have introduced alternative syntaxes, like:

actual.Should().Be(expected);

Or similar. Which is certainly a lot clearer than assertEquals but also much better than assertExpectedEqualsActual. And it is also a lot more composable.

  • 1
    I am anal and I do follow the recommended order but it seems to me that if I expect the result of fun(x) to be 5 then what could go wrong if reverse the order - assert(fun(x), 5)? How did it bite you? – emory Jul 1 '18 at 11:03
  • 3
    @emory I know jUnit (at least) builds a thourough error message from the values of expected and actual, so reversing them might result in a message not accurate. But I agree that it sounds more natural though :) – joH1 Jul 1 '18 at 17:16
  • @joH1 it seems weak to me. failing code will fail and passing code will pass whether you do assert(expected, observed) or assert(observed, expected). A better example would be something like locateLatitudeLongitude - if you reverse the coordinates, it will seriously mess up. – emory Jul 1 '18 at 23:47
  • 1
    @emory People that don't care about sensible error messages in unit tests are why I have to deal with "Assert.IsTrue failed" in some old code bases. Which is tremendous fun to debug. But yes in this case the problem might not be that essential (except if we do fuzzy comparisons where the order of arguments generally matters). Fluent assertions are indeed a great way to avoid this problem and also make the code way more expressive (and provide much better error message to boot). – Voo Jul 2 '18 at 12:55
  • @emory: Reversing the argument will make the error messages misleading, and send you down the wrong path when debugging. – JacquesB Jul 4 '18 at 10:11
5

You are trying to steer your path between Scylla and Charybdis to clarity, trying to avoid useless verbosity (also known as aimless rambling) as well as excessive brevity (also known as cryptic terseness).

So, we have to look at the interface you want to evaluate, a way to do debug-assertions that two objects are equal.

  1. Is there any other function it could be considering arity and name?
    No, so the name itself is clear enough.
  2. Are the types of any significance?
    No, so let's ignore them. You did that already? Good.
  3. Is it symmetric in its arguments?
    Nearly, on error the message puts each arguments representation into their own spot.

So, let's see whether that small difference is of any significance, and not covered by existing strong conventions.

Is the intended audience inconvenienced if the arguments are unintendedly swapped?
No, the developers also get a stack-trace and they have to scrutinize the source-code anyway to fix the bug.
Even without a full stack-trace, the assertions position resolves that question. And if even that is missing and it's not obvious from the message which is which, it at most doubles the possibilities.

Does the arguments order follow convention?
Seems to be the case. Though it seems at best a weak convention.

Thus, the difference looks quite insignificant, and the argument-order is covered by strong-enough convention that any effort to put encode it into the function-name has negative utility.

  • well the order might matter with jUnit, which builds a specific error message from the values of expected and actual (at least with Strings) – joH1 Jul 1 '18 at 17:19
  • I think I covered that part... – Deduplicator Jul 1 '18 at 17:49
  • you mentioned it but consider: assertEquals("foo", "doo") gives the error message is ComparisonFailure: expected:<[f]oo> but was:<[d]oo>... Swapping the values would invert the meaning of the message, that sounds more antisymmetric to me. Anyway like you said a dev has other indicators to resolve the error, but it can be misleading IMHO and take a bit more debugging time. – joH1 Jul 2 '18 at 6:27
  • The idea that there is a "convention" for argument orders is funny, considering that both camps (dest, src vs. src, dest) have argued about this for at least as long as AT&T vs. Intel syntax has existed. And unhelpful error messages in unit tests are a plague that should be eradicated not enforced. That's almost as bad as "Assert.IsTrue failed" ("hey you have to execute the unit test anyhow to debug it, so just run it again and put a breakpoint there", "hey you have to look at the code anyhow, so just check if the order is correct"). – Voo Jul 2 '18 at 12:57
  • @Voo: The point is that the "damage" for getting it wrong is miniscule (the logic does not depend on it, and the messages utility is not impaired to any significant degree), and when writing the IDE will show you the parameters name and type anyway. – Deduplicator Jul 2 '18 at 13:31
3

Often it does not add any logical clarity.

Compare "Add" to "AddFirstArgumentToSecondArgument".

If you need an overload that, say, adds three values. What would make more sense?

Another "Add" with three arguments?

or

"AddFirstAndSecondAndThirdArgument"?

The name of the method should convey its logical meaning. It should tell what it does. Telling, on a micro-level, what steps it takes does not make it easier for the reader. The names of the arguments will provide additional detail if needed. If you need more detail still, the code will be right there for you.

  • 4
    Add suggests a commutative operation. The OP is concerned with situations where the order matters. – Rosie F Jun 30 '18 at 21:19
  • In Swift, you would for example call add (5, to: x) or add (5, plus: 7, to: x) or add (5, plus: 7, giving: x) if you define the add() function accordingly. – gnasher729 Jun 30 '18 at 23:56
  • The third overload should be named "Sum" – StingyJack Jul 1 '18 at 12:26
  • @StringyJack Hmm.. Sum is not an instruction, it is a noun which makes it less suitable for a method name. But if you feel that way and If you want to be a purist about it, the two argument version should also be named Sum. If you were to have an Add method, it should have one argument that is added to the object instance itself (which would have to be a numeric or vector type). The 2 or more argument varieties (whatever you would name them) would be static. Then the 3 or more argument versions would be redundant and we would have implemented a plus operator :-| – Martin Maat Jul 1 '18 at 12:52
  • 1
    @Martin Wait what? sum is a perfectly cromulent verb. It's particularly common in the phrase "to sum up". – Voo Jul 2 '18 at 12:59
2

I'd like to add something else that's hinted at by other answers, but I don't think has been mentioned explicitly:

@puck says "There still is no guarantee the first mentioned argument in the function name really is the first parameter."

@cbojar says "Use types instead of ambiguous arguments"

The issue is that programming languages don't understand names: they're just treated as opaque, atomic symbols. Hence, like with code comments, there is not necessarily any correlation between what a function is named and how it actually operates.

Compare assertExpectedEqualsActual(foo, bar) with some alternatives (from this page and elsewhere), like:

# Putting the arguments in a labelled structure
assertEquals({expected: foo, actual: bar})

# Using a keyword arguments language feature
assertEquals(expected=foo, actual=bar)

# Giving the arguments different types, forcing us to wrap them
assertEquals(Expected(foo), Actual(bar))

# Breaking the symmetry and attaching the code to one of the arguments
bar.Should().Be(foo)

These all have more structure than the verbose name, which gives the language something non-opaque to look at. The definition and usage of the function also depends on this structure, so it can't get out-of-sync with what the implementation is doing (like a name or comment can).

When I encounter or forsee a problem like this, before I shout at my computer in frustration I first take a moment to ask whether it's 'fair' to blame the machine at all. In other words, was the machine given enough information to distinguish what I wanted from what I asked for?

A call like assertEqual(expected, actual) makes as much sense as assertEqual(actual, expected), so it's easy for us to get them mixed up and for the machine to plough ahead and do the wrong thing. If we used assertExpectedEqualsActual instead, it might make us less likely to make a mistake, but it gives no more information to the machine (it can't understand English, and choice of name should not affect semantics).

What makes the "structured" approaches more preferable, like keyword arguments, labelled fields, distinct types, etc. is that the extra information is also machine readable, so we can have the machine spot incorrect usages and help us do things right. The assertEqual case isn't too bad, since the only problem would be inaccurate messages. A more sinister example might be String replace(String old, String new, String content), which is easy to confuse with String replace(String content, String old, String new) which has a very different meaning. A simple remedy would be to take a pair [old, new], which would make mistakes trigger an error immediately (even without types).

Note that even with types, we may find ourselves not 'telling the machine what we want'. For example the anti-pattern called "stringly typed programming" treats all data as strings, which makes it easy to get arguments mixed up (like this case), to forget to perform some step (e.g. escaping), to accidentally break invariants (e.g. making unparseable JSON), etc.

This is also related to "boolean blindness", where we calculate a bunch of booleans (or numbers, etc.) in one part of the code, but when trying to use them in another it's not clear what they're actually representing, whether we've got them mixed up, etc. Compare this to e.g. distinct enums which have descriptive names (e.g. LOGGING_DISABLED rather than false) and which cause an error message if we get them mixed up.

1

because it removes the need to remember where the arguments go

Does it really? There still is no guarantee the first mentioned argument in the function name really is the first parameter. So better look it up (or let your IDE do that) and stay with reasonable names than blindly rely on a quite silly name.

If you read the code you should easily see what happens when the parameters are named as they should be. copy(source, destination) is much easier to understand than soemthing like copyFromTheFirstLocationToTheSecondLocation(placeA, placeB).

Why don't coders adopt the former if it is, as the author asserts, clearer than the latter?

Because there are different points of view on different styles and you can find x authors of other articles that state the opposite. You'd go insane trying to follow everything somebody writes somewhere ;-)

0

I agree that encoding parameter names into function names makes writing and using functions more intuitive.

copyFromSourceToDestination( // "...ahh yes, the source directory goes first"

It's easy to forget the ordering of arguments in functions and shell commands and many programmers rely on IDE features or function references for this reason. Having the arguments described in the name would be an eloquent solution to this dependency.

However once written, the description of the arguments becomes redunant to the next programmer who has to read the statement, since in most cases named variables will be used.

copy(sourceDir, destinationDir); // "...makes sense"

The terseness of this will win over most programmers and I personally find it easier to read.

EDIT: As @Blrfl pointed out, encoding parameters isn't that 'intuitive' after all since you need to remember the name of the function in the first place. This requires looking up function references or getting help from an IDE which will likely provide parameter ordering information anyway.

  • 9
    So if I can play devil's advocate for a minute: It's only intuitive when you know the full name of the function. If you know there's a copy function and you don't remember whether it's copyFromSourceToDestination or copyToDestinationFromSource, your choices are finding it by trial and error or reading the reference material. IDEs that can complete partial names are just an automated version of the latter. – Blrfl Jun 30 '18 at 14:57
  • @Blrfl The point of calling it copyFromSourceToDestination is that if you think it's copyToDestinationFromSource, the compiler will find your bug, but if it was called copy, it won't. Getting a copy-routine's params the wrong way round is easy, since strcpy, strcat etc. set a precedent. And is the terse one easier to read? Does mergeLists(listA, listB, listC) create listA from listB & listC, or read listA & listB and write listC? – Rosie F Jun 30 '18 at 21:18
  • 4
    @RosieF If I wasn't positive what the arguments meant, I'd be reading the documentation before writing the code. Besides, even with the more-verbose function names, there's still room for interpretation about what the order actually is. Someone taking a cold look at the code won't be able to intuit that you've established the convention that what's in the function's name reflects the order of the arguments. They'd still have to know it ahead of time or read the docs. – Blrfl Jul 1 '18 at 10:37
  • OTOH, destinationDir.copy(sourceDir); // "...makes more sense" – Kristian H Jul 1 '18 at 12:29
  • 1
    @KristianH What direction does dir1.copy(dir2) work? No idea. What about dir1.copyTo(dir2)? – maaartinus Jul 2 '18 at 15:06

Not the answer you're looking for? Browse other questions tagged or ask your own question.