API Design - pre-mix index validation beyond borders?

Question

I am developing an API that does something like this:

// Drop $howMany items from after $from def dropElements[T](array: Array[T], from: Int, howMany: Int) 

The expected behavior is that howMany should be non-negative, and if howMany is zero, it should not make any changes. I have two ways to implement this:

 def dropElements[T](array: Array[T], from: Int, howMany: Int) { assert(howMany >= 0); if (howMany == 0) return; assert(0 <= from && from < array.length); .... } 

OR

 def dropElements[T](array: Array[T], from: Int, howMany: Int) { assert(howMany >= 0); assert(0 <= from && from < array.length); if (howMany == 0) return; .... } 

I stand for the second approach (declaring your preconditions in advance) compared to the first approach, but I was told that the first approach is more respectful with the requirements when howMany = 0.

Any thoughts or pros and cons? I ask how the constructor of the standard collection library

Answer 1

My thoughts are what it stands for:

I think it is more consistent to perform a check from bindings in all cases. The out-of-date from is probably an error in the caller’s code, regardless of howMany value. For me, this is preferable in this case, unsuccessful.

I do not see this as a violation of the requirements. At least I (as a likely future user of your api) would not be surprised by this behavior.

In addition, as you indicate, the preconditions are readable in advance.

So the second approach is for me.

Answer 2

Your question and example will lead to a number of problems.

Do you really want to quit?

The Scala standard library approaches a certain length, trying to accommodate everything that the client passes as index arguments. In many cases, a negative index is interpreted as zero, and anything outside the collection size is simply ignored. See drop() and take() for examples.

Moreover, if you are going to scold the client for bad argument values, then it makes sense to test all the arguments received, even if one or the other becomes insignificant for the result.

assert() or require() ?

assert() has some advantages over require() , but the latter seems more suitable for the use you got here. You can read this for more information on the topic.

Rethink the wheel.

Scala already offers a dropElements method called patch .

 def dropElements[T](array: Array[T], from: Int, howMany: Int) = { array.patch(from, Seq(), howMany) } 

Except that it returns ArraySeq[T] instead of Array[T] . Arrays in Scala can be a bit of a pain this way.

strengthen-my-library

To make the new method look and look more Scala -like, you can "add" it to the Array library.

 implicit class EnhancedArray[T](arr: Array[T]) { def dropElements(from: Int, howMany: Int): Array[T] = { arr.drop(from+howMany).copyToArray(arr,from) arr.dropRight(howMany) } } Array(1,1,1,8,8,7).dropElements(3,2) // res0: Array[Int] = Array(1, 1, 1, 7) 

Answer 3

First of all, I recommend using require over assert for preconditions. Indeed, require intended to (and probably specifically added to) the prerequisites, as described in the "Claims" section of the documentation ). In addition, assert calls can be removed if your code is compiled with -Xdisable-assertions . This may not be a big problem in this particular case, but if the implementation depends on certain preconditions that can be implemented, bad things can happen. If you (or users of your function) use a static analysis tool, it is probably better to use require than assert s.

I would prefer a second approach for several reasons.

Strictly speaking, you can only provide a precondition if you check it at the beginning. In the first approach, your precondition is actually howMany >= 0 && (howMany == 0 || (0 <= from && from < array.length)) instead of howMany >= 0 && (0 <= from && from < array.length) .

Why do you prefer the latter over the former? If the caller passes outside of from , it is unlikely that there is an error in the caller's code. Admittedly, I am speculating here, as there are many potential use cases for your function. But if someone comes up with a convincing precedent for passing indices outside the limits that outweigh the opposing arguments, you can still mitigate the precondition later. Another way to break compatibility.

You say that "if howMany is zero, it should not make any changes." This follows from the supposed semantics of your function. If you implement it as an early return or not, this is an implementation detail, therefore, in my opinion, the line if (howMany == 0) return; should be a part .... in your question, and then the question itself is responsible for the second approach.

Answer 4

I prefer to kill two birds with one stone: avoiding return , putting the body of the method in one else clause and putting all the statements together. In addition, statements, if possible, may fail if possible:

 def dropElements[T](array: Array[T], from: Int, howMany: Int): Array[T] = if (howMany == 0) array else { assert(howMany > 0, "howMany must > 0") assert(from >= 0, "from must >= 0") assert(from < array.length, s"from must < ${array.length}") ... } 

Answer 5

My 2 cents: in Scala, I usually try to avoid using such if with return (s) buried in the middle of a method call (of course, this method is short, but in longer ... debugging becomes complicated): there are cleaner, better ways do it. Also, I would be proactive and check if the array you get is null.

I would do something like this

 def dropElements[T](array: Array[T], from: Int, howMany: Int): Option(array) match{ case Some(arr) if arr.nonEmpty => // here 'arr' is guaranteed to be non-null, and with at least one element require(howMany > 0, "howMany must > 0") require(from >= 0, "from must >= 0") require(from < array.length, s"from must < ${array.length}") // rest of the code here case _ => // do nothing if the array is null, or it is empty }