Parameter Validation


(David Hart) #1

When writing library code, what method of parameter validation would be suggested?

Assert

func greetUser(user: String, times: Int) {
  assert(user.count > 0)
  assert(times > 0)

  for _ in 0..<times {
    print(“Hello \(user)”)
  }
}

Pros
* Easy to use

Cons
* Disabled in release
* Not unit-testable

Error-Handling

Pros
* Unit-testable

Cons
* Quickly litters all calling code with try's

In the case Error-Handling is used, what kind of ErrorType would you suggest?

enum GreetUserError : ErrorType {
  case UserEmpty
  case TimesInvalid
}

func greetUser(user: String, times: Int) throws {
  guard user.count > 0 else {
    throw GreetUserError.UserEmpty
  }

  guard times > 0 else {
    throw GreetUserError.TimesInvalid
  }

  for _ in 0..<times {
    print(“Hello \(user)”)
  }
}

Or should more generic re-usable errors be used? But less descriptive errors except if messages are provided every times:

enum ParameterError : ErrorType {
  case Empty(String)
  case NotInRange(String)
}

func greetUser(user: String, times: Int) throws {
  guard user.count > 0 else {
    throw ParameterError.Empty(“user")
  }

  guard times > 0 else {
    throw ParameterError.NotInRange(“times")
  }

  for _ in 0..<times {
    print(“Hello \(user)”)
  }
}


(Jens Alfke) #2

When writing library code, what method of parameter validation would be suggested?

Definitely assert. Assert is for illegal calls, i.e. programmer errors, while errors (throw) are for valid runtime error conditions.

Cons
* Disabled in release

I believe there is a compiler flag to control this, if you want them enabled in the release.

* Not unit-testable

Well, you can’t create tests for rejection of illegal parameters. Which doesn’t seem like a huge loss to me. It would be nice to be able to do this, though. Maybe through some mechanism for catching / recovering from the otherwise-fatal exception triggered by an assert failure?

—Jens

···

On Dec 6, 2015, at 12:49 PM, David Hart via swift-users <swift-users@swift.org> wrote:


(Brent Royal-Gordon) #3

When writing library code, what method of parameter validation would be suggested?

It depends.

If the data may come from somewhere out of your control, like a user or the network, throws (or an optional or Bool return value) is an appropriate choice.

If the data will come from a source which should “never” be wrong, like data that has already been parsed or a calculation you perform, then precondition() (rather than assert(), which is removed in production builds) is a good way to run a sanity check.

Basically, if you anticipate that the value could ever be invalid, use throws or an optional/boolean return. If you don’t think the value should ever be invalid, use precondition().

···

--
Brent Royal-Gordon
Architechies


(David Hart) #4

In that case, we really need a way to unit-test assertions, because unit-testing illegal parameters in a library seems critical.
David.

···

On 07 Dec 2015, at 06:33, Jens Alfke <jens@mooseyard.com> wrote:

On Dec 6, 2015, at 12:49 PM, David Hart via swift-users <swift-users@swift.org <mailto:swift-users@swift.org>> wrote:

When writing library code, what method of parameter validation would be suggested?

Definitely assert. Assert is for illegal calls, i.e. programmer errors, while errors (throw) are for valid runtime error conditions.

Cons
* Disabled in release

I believe there is a compiler flag to control this, if you want them enabled in the release.

* Not unit-testable

Well, you can’t create tests for rejection of illegal parameters. Which doesn’t seem like a huge loss to me. It would be nice to be able to do this, though. Maybe through some mechanism for catching / recovering from the otherwise-fatal exception triggered by an assert failure?

—Jens


(Jason Dusek) #5

Say for a moment we wanted to capture every such constraint at type
level -- with types NonEmptyString and PositiveNonZeroInteger. Would
these be declared as structs in Swift?


(David Hart) #6

I’m looking at the special case of library code. If I surface an API in a library, it’s the library user who will call this function. Would you regard this as an assert or throws scenario?

···

On 07 Dec 2015, at 08:25, Brent Royal-Gordon via swift-users <swift-users@swift.org> wrote:

When writing library code, what method of parameter validation would be suggested?

It depends.

If the data may come from somewhere out of your control, like a user or the network, throws (or an optional or Bool return value) is an appropriate choice.

If the data will come from a source which should “never” be wrong, like data that has already been parsed or a calculation you perform, then precondition() (rather than assert(), which is removed in production builds) is a good way to run a sanity check.

Basically, if you anticipate that the value could ever be invalid, use throws or an optional/boolean return. If you don’t think the value should ever be invalid, use precondition().

--
Brent Royal-Gordon
Architechies

_______________________________________________
swift-users mailing list
swift-users@swift.org
https://lists.swift.org/mailman/listinfo/swift-users


(Brent Royal-Gordon) #7

I’m looking at the special case of library code. If I surface an API in a library, it’s the library user who will call this function. Would you regard this as an assert or throws scenario?

“Library” is not a special case here—the same basic considerations apply. If you expect API users to pass you raw data from an untrustworthy source, throw. If you expect them to always give you valid data, use precondition().

For example, let’s say you’ve been hired by a company called PayMe to write PayMeKit.

* PayMeServer.setDeveloperToken(_:slight_smile: takes a token issued by PayMe to each developer, which should be hardcoded into the app. This API should use precondition() to check if the token is valid.
* PayMeCard.init(name:cardNumber:expirationMonth:year:) creates an object from data which is probably provided by the user. This API should throw if the data is invalid.
* PayMeServer.buyProductWithID(_:usingCard:) is an interesting case. The product ID is probably developer-controlled, and presumably should always be valid, so we’ll precondition() that. But suppose the product ID is valid but the product is out of stock. We don’t want to crash the app, right? So we’ll throw for that one.

(Of course, buyProductWithID would in practice probably be an async call with a completion handler, and instead of throwing, you’d pass an ErrorType to the completion handler. But you get the idea.)

In short: Even when you’re writing a library, you can’t escape the requirement that you think about how your product will be used. Sorry that there’s no single answer I can give.

···

--
Brent Royal-Gordon
Architechies


(Jens Alfke) #8

Assert. It’s not library vs. non-library, it’s a question of bug vs. legitimate runtime error. Passing an invalid parameter value is a bug.

For example, Foundation is a library, and passing an invalid parameter to a Foundation method triggers an assertion failure.

—Jens

···

On Dec 6, 2015, at 11:49 PM, David Hart via swift-users <swift-users@swift.org> wrote:

I’m looking at the special case of library code. If I surface an API in a library, it’s the library user who will call this function. Would you regard this as an assert or throws scenario?


(David Hart) #9

But then you can't unit-test that the function fails on those parameters. To look at a counter-argument, both C# and Java would throw exceptions for parameter validation.

···

On 07 Dec 2015, at 17:30, Jens Alfke <jens@mooseyard.com> wrote:

On Dec 6, 2015, at 11:49 PM, David Hart via swift-users <swift-users@swift.org> wrote:

I’m looking at the special case of library code. If I surface an API in a library, it’s the library user who will call this function. Would you regard this as an assert or throws scenario?

Assert. It’s not library vs. non-library, it’s a question of bug vs. legitimate runtime error. Passing an invalid parameter value is a bug.

For example, Foundation is a library, and passing an invalid parameter to a Foundation method triggers an assertion failure.

—Jens


(Jens Alfke) #10

I agree, that’s a drawback. But the alternative of ‘throwing’ an exception adds so much overhead that it’s IMHO a non-starter. Except in the case where the method can already return/throw errors, but that’s not always going to be the case. Operator overloads are a particular problem for errors — consider range-checking the index to a subscript operator: you have no choice but an assertion, since that method is incapable of returning/throwing errors.

As I said yesterday, I think the best solution is to make the unit testing framework somehow capable of magically recovering from assertion failure. (Which it might be already, at least on Apple platforms — I haven’t explicitly tested what XCTest does, but I don’t remember my test suites ever completely bombing out due to an assertion failure. IIRC they just continue to the next test. But it’s possible I’m forgetting!)

—Jens

···

On Dec 7, 2015, at 9:03 AM, David Hart <david@wittywings.fr> wrote:

But then you can't unit-test that the function fails on those parameters. To look at a counter-argument, both C# and Java would throw exceptions for parameter validation.


(Dmitri Gribenko) #11

Yes you can. Maybe not with the current XCTest, but there's nothing that
prevents unit-testing traps in principle. The standard library is already
doing that. See test/1_stdlib/ArrayTraps.swift.gyb for some examples:

ArrayTraps.test("downcast1")
  .skip(.Custom(
    { _isFastAssertConfiguration() },
    reason: "this trap is not guaranteed to happen in -Ounchecked"))
  .code {
  let ba: [Base] = [ Derived(), Base() ]
  let da = ba as! [Derived]
  let d0 = da[0]
  expectCrashLater()
  da[1]
}

Dmitri

···

On Mon, Dec 7, 2015 at 9:03 AM, David Hart via swift-users < swift-users@swift.org> wrote:

But then you can't unit-test that the function fails on those parameters.

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/


(Jens Alfke) #12

Nice! But completely undocumented, AFAIK. This is the first I’ve heard of the term “trap” or a way to handle them. The docs I’ve seen just say that an assertion failure terminates the process.

Is there any more info about traps, other than “Read the source, Luke?” :wink:

(This is one reason I’ve been excited for the open source release — finally getting to look behind the scenes of some of the “magic” stuff.)

—Jens

···

On Dec 7, 2015, at 9:21 AM, Dmitri Gribenko <gribozavr@gmail.com> wrote:

Yes you can. Maybe not with the current XCTest, but there's nothing that prevents unit-testing traps in principle. The standard library is already doing that. See test/1_stdlib/ArrayTraps.swift.gyb for some examples:


(Dmitri Gribenko) #13

Yes you can. Maybe not with the current XCTest, but there's nothing that
prevents unit-testing traps in principle. The standard library is already
doing that. See test/1_stdlib/ArrayTraps.swift.gyb for some examples:

Nice! But completely undocumented, AFAIK. This is the first I’ve heard of
the term “trap”

We don't call them "crashes", because the word "crash" is typically
used to describe non-reliable program termination because of an
invalid memory unsafe operation that lead to memory corruption. Traps
are guaranteed to happen, and they *prevent* memory unsafe operations
to happen, terminating the process before they happen.

or a way to handle them. The docs I’ve seen just say that an
assertion failure terminates the process.

Traps do terminate the process, without any way to recover, and that
is exactly what is happening in the standard library tests. There are
just two processes -- one is performing a trap, and another one is the
test harness that verifies that the other process terminated.

Is there any more info about traps, other than “Read the source, Luke?” :wink:

What would you like to know?

Dmitri

···

On Mon, Dec 7, 2015 at 9:25 AM, Jens Alfke <jens@mooseyard.com> wrote:

On Dec 7, 2015, at 9:21 AM, Dmitri Gribenko <gribozavr@gmail.com> wrote:

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/


(Jens Alfke) #14

There are
just two processes -- one is performing a trap, and another one is the
test harness that verifies that the other process terminated.

Ah — I thought it was some type of in-process exception/signal handler.

What would you like to know?

Given the above, not much :slight_smile: I suppose an assertion failure just raises a SIGABORT or something similar?

—Jens

···

On Dec 7, 2015, at 9:40 AM, Dmitri Gribenko <gribozavr@gmail.com> wrote:


(Dmitri Gribenko) #15

There are
just two processes -- one is performing a trap, and another one is the
test harness that verifies that the other process terminated.

Ah — I thought it was some type of in-process exception/signal handler.

Well, if we used that, then we wouldn't be testing in the same
environment as production code would run in.

Even more, when a compiler sees that the process is going to trap
anyway, it can omit some cleanups, like ARC releases, which will lead
to memory leaks and other surprising behavior.

While it is technically possible to intercept the control flow after a
Swift trap, I would not recommend doing that even for test purposes,
and even less for production code, for the reasons I described.

What would you like to know?

Given the above, not much :slight_smile: I suppose an assertion failure just raises a
SIGABORT or something similar?

It performs a UD2 instruction on x86, or equivalent on other
platforms. stdlib/public/core/AssertCommon.swift if you are
interested.

Dmitri

···

On Mon, Dec 7, 2015 at 10:02 AM, Jens Alfke <jens@mooseyard.com> wrote:

On Dec 7, 2015, at 9:40 AM, Dmitri Gribenko <gribozavr@gmail.com> wrote:

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/


#16

But what overhead? As you know Swift has *NO* exceptions. It’s just syntax sugar for normale error values.

Jan

···

On 07.12.2015, at 18:18, Jens Alfke via swift-users <swift-users@swift.org> wrote:

On Dec 7, 2015, at 9:03 AM, David Hart <david@wittywings.fr <mailto:david@wittywings.fr>> wrote:

But then you can't unit-test that the function fails on those parameters. To look at a counter-argument, both C# and Java would throw exceptions for parameter validation.

I agree, that’s a drawback. But the alternative of ‘throwing’ an exception adds so much overhead that it’s IMHO a non-starter. Except in the case where the method can already return/throw errors, but that’s not always going to be the case. Operator overloads are a particular problem for errors — consider range-checking the index to a subscript operator: you have no choice but an assertion, since that method is incapable of returning/throwing errors.

As I said yesterday, I think the best solution is to make the unit testing framework somehow capable of magically recovering from assertion failure. (Which it might be already, at least on Apple platforms — I haven’t explicitly tested what XCTest does, but I don’t remember my test suites ever completely bombing out due to an assertion failure. IIRC they just continue to the next test. But it’s possible I’m forgetting!)

—Jens

_______________________________________________
swift-users mailing list
swift-users@swift.org
https://lists.swift.org/mailman/listinfo/swift-users


(Jens Alfke) #17

Mostly the overhead in the caller of having to pass a (hidden) error parameter into the call, and check the return value and branch afterwards.

(C++/Obj-C exceptions actually have no runtime cost in the normal success case; it’s only throwing an exception that’s expensive.)

—Jens

···

On Dec 7, 2015, at 10:56 AM, Jan Neumüller via swift-users <swift-users@swift.org> wrote:

But what overhead? As you know Swift has *NO* exceptions. It’s just syntax sugar for normale error values.


#18

I fail to see any overhead here. Its the absolute minimum to get checked values. Anything less is not checking.

Jan

···

On 07.12.2015, at 20:03, Jens Alfke <jens@mooseyard.com> wrote:

On Dec 7, 2015, at 10:56 AM, Jan Neumüller via swift-users <swift-users@swift.org <mailto:swift-users@swift.org>> wrote:

But what overhead? As you know Swift has *NO* exceptions. It’s just syntax sugar for normale error values.

Mostly the overhead in the caller of having to pass a (hidden) error parameter into the call, and check the return value and branch afterwards.


(Jens Alfke) #19

There’s no overhead if the function already ‘throws’ / returns errors. But not all functions do that (and some functions _cannot_ do that, like operators.) If a function has to be upgraded to return an error, that _is_ extra overhead, mostly on the caller’s side. (And, as previously noted, this may complicate the programmer’s life too, because now they may have to either propagate ‘throws’ to the caller or add a ‘catch’ handler.)

Anyway, the programming conventions have been clear for a very long time: assertions should be used for programmer errors (like invalid parameters) while returned errors should be used for legitimate runtime error situations (like file-not-found). This predates Swift and even predates Mac OS X*; I think it goes back to OpenStep in the mid-‘90s.

—Jens

* NSError itself isn’t that old, but before that the APIs still returned nil or false for runtime errors.

···

On Dec 7, 2015, at 11:39 AM, Jan Neumüller via swift-users <swift-users@swift.org> wrote:

I fail to see any overhead here. Its the absolute minimum to get checked values. Anything less is not checking.


(David Hart) #20

You enable assertions for release builds then? Remember, I'm talking specifically about library code: the same way that calling NSArray's objectAtIndex throws an exception (if my memory serves well) for out of bounds indices. At least, NSAssert in Objective-C throws exceptions and is catchable.

···

On 08 Dec 2015, at 01:43, Jens Alfke via swift-users <swift-users@swift.org> wrote:

On Dec 7, 2015, at 11:39 AM, Jan Neumüller via swift-users <swift-users@swift.org> wrote:

I fail to see any overhead here. Its the absolute minimum to get checked values. Anything less is not checking.

There’s no overhead if the function already ‘throws’ / returns errors. But not all functions do that (and some functions _cannot_ do that, like operators.) If a function has to be upgraded to return an error, that _is_ extra overhead, mostly on the caller’s side. (And, as previously noted, this may complicate the programmer’s life too, because now they may have to either propagate ‘throws’ to the caller or add a ‘catch’ handler.)

Anyway, the programming conventions have been clear for a very long time: assertions should be used for programmer errors (like invalid parameters) while returned errors should be used for legitimate runtime error situations (like file-not-found). This predates Swift and even predates Mac OS X*; I think it goes back to OpenStep in the mid-‘90s.

—Jens

* NSError itself isn’t that old, but before that the APIs still returned nil or false for runtime errors.

_______________________________________________
swift-users mailing list
swift-users@swift.org
https://lists.swift.org/mailman/listinfo/swift-users