dart-archive / angular.dart

Legacy source repository. See github.com/dart-lang/angular
https://webdev.dartlang.org/angular/
1.25k stars 248 forks source link

mock/http_backend should not throw when requests is empty #1676

Open trinarytree opened 9 years ago

trinarytree commented 9 years ago

right now, mock/http_backend.dart's flush method is implemented so that if requests is empty, it will throw. the preceding comment says "If there are no pending requests when the flush method is called an exception is thrown (as this typically a sign of programming error)" but i don't agree. i could make the same spurious argument to claim that adding 0 to a number ought to be an error or clearing an empty list ought to be an error.

why would you ever add 0 to a number? well, usually because you are abstractly adding the value a variable is bound to and it just happens to be bound to 0. it's important not to make 0 a special case, otherwise mathematics would be full of annoying case logic, e.g. how annoying is this?: "the number of sheep we have is the number of sheep we had yesterday minus the number we lent to bob, unless we didn't lend him any, plus the number carol paid back to us, unless she didn't pay any back to us, in which case, it's just (the number we had yesterday minus the number we lent to bob unless we didn't lend him any)." or in code: /// stupid people think adding 0 is stupid numSheepToday = numReturnedByCarol > 0 ? (numLentToBob > 0 ? numSheepYesterday - numLentToBob : numSheepYesterday) + numReturnedByCarol : (numLentToBob > 0 ? numSheepYesterday - numLentToBob : numSheepYesterday); /// smart people think adding 0 is smart numSheepToday = numSheepYesterday - numLentToBob + numReturnedByCarol; the (incorrect) argument for why you shouldn't be allowed to add 0 to a number goes like this: "if you wanted to add 0, just don't add the number. and you certainly know in advance whether you are adding 0, right?" that's the same flawed logic being used to justify throwing when flush has nothing to flush. there is this mistaken belief that you ought to know in advance that there is nothing to flush. but you _don't_ necessarily know in advance, e.g. if your code is inside a testing framework.

obviously, one can work around this with if (backend.requests.isNotEmpty) { backend.flush(); } but this is just undoing what i think shouldn't have been done in the first place and it now requires a boring comment justifying why it's circumventing angular's check.

possible fixes: (1) just get rid of the check (my vote). (2) make the check optional. but what should the default be? if the idea is that frameworks should be allowed to be oblivious about how many requests are pending but app level code should "know better", then it would seem that the default should be to have the check. you can't have an optional named param because of a silly dart limitation where you can't have both optional positional and named params for the same method. so another solution would be to have 2 methods, one delegating to the other. e.g. flush and flushIdempotent (or flushLenient. feel free to think of a better name). (3) status quo. force frameworks to add the guard and a boring comment.