swift-nio: `ByteBuffer.set(string:)` returns an `Optional` Int

Expected behavior

ByteBuffer.set(string: String, at index: Int) -> Int

The set(string:at:) (and write(string:)) return an Optional<Int> even though they never fail (looking at the implementation). Presumably (and talking to @weissi ) this is a left-over from an old variant of the func which supported different encodings.

Also: the comment doesn’t say anything about the optionality, making it even more confusing. Does it mean no buffer space is left? Does it mean the encoding failed? Does it mean Mercury passed Venus?

Since the buffer is auto-expanding, I think this should just not fail and return a plain Int.

Actual behavior

ByteBuffer.set(string: String, at index: Int) -> Int?

SwiftNIO version/commit hash

Head of today, shouldn’t matter.

Swift & OS version (output of swift --version && uname -a)

Any.

About this issue

  • Original URL
  • State: closed
  • Created 6 years ago
  • Comments: 15 (9 by maintainers)

Commits related to this issue

Most upvoted comments

My feeling is that they don’t want you to overload the method name like that. The base name should denote what it does, and if it is overloading, just the type.

I’m looking primarily at the “Compensate for weak type information” of the API Design Guidelines. Look how they do addObserver(...) instead of add(observer:).

Or makeWidget(_...) not make(widget:...) in the same document.

But I might be completely wrong on this, I also walked over this naming issue a few times myself and wasn’t quite sure. Though I think your own example is actually pretty good. It should be this (I guess):

func writeString(_ s: UnsafePointer<UInt8>)
func writeString(_ s: StaticString)
func writeString(_ s: String)

What you do is you write a string. What you write can come from various value types and the type could be different. It would even cover my case:

func writeString(_ i: Integer, radix: Int = 10)

Which kinda shows the thinking.

Again, I might be really wrong on this. The only practical issue w/ the current style is that Xcode is really bad at completing overloaded base names, but that is more of an Xcode issue.

@weissi If the error can be meaningful, the function should be throwing. The combination of throws and @discardableResult should give exactly what you describe: possibility to ignore the resulting Int value, but require attention to possible error.