Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate API to a more idiomatic naming scheme #148

Open
regexident opened this issue Oct 13, 2019 · 1 comment
Open

Migrate API to a more idiomatic naming scheme #148

regexident opened this issue Oct 13, 2019 · 1 comment
Assignees
Milestone

Comments

@regexident
Copy link
Collaborator

regexident commented Oct 13, 2019

tl;dr

Surge's API naming scheme does not quite follow Swift's API Design Guidelines.
We might want to fix that.

What

Swift and its stdlib have gone through several major changes since its initial release in 2014 (Surge's initial commit was shortly after, on Aug 24, 2014).

The most notable change probably was the migration to the then new and more idiomatic naming scheme with Swift 3: SE-0005, which resulted in the API Design Guidelines.

The most relevant part of them being:

Name functions and methods according to their side-effects

  • Those without side-effects should read as noun phrases, e.g. > x.distance(to: y), i.successor().

  • Those with side-effects should read as imperative verb phrases, > e.g., print(x), x.sort(), x.append(y).

  • Name Mutating/nonmutating method pairs consistently. A mutating > method will often have a nonmutating variant with similar semantics, > but that returns a new value rather than updating an instance > in-place.

    • When the operation is naturally described by a verb, use the > verb’s imperative for the mutating method and apply the “ed” or > “ing” suffix to name its nonmutating counterpart.

      Mutating Nonmutating
      x.sort() z = x.sorted()
      x.append(y) z = x.appending(y)
    • When the operation is naturally described by a noun, use the > noun for the nonmutating method and apply the “form” prefix to > name its mutating counterpart.

      Nonmutating Mutating
      x = y.union(z) y.formUnion(z)
      j = c.successor(i) c.formSuccessor(&i)

Why

As @mattt and I discussed in more depth in #107 the current naming scheme of our public, as well as internal API does not quite fit the idiomatic naming scheme as established with Swift 3.

Surge in its current state has the following naming scheme:

Mutating Nonmutating
addInPlace(&a, b) z = add(a, b)

Note: For the sake of simplicity I will refer to T: Unsafe(Mutable)MemoryAccessible as simply [Scalar] for the remainder of this writing.

Surge contains APIs for working with low-level scalar-buffers ([Scalar]), as well as APIs for working with high-level vectors (Vector<Scalar>) and matrices (Matrix<Scalar>). While both APIs are related by the fact of the latter being implemented in terms of the former, from a user's perspective they can be considered mostly unrelated, i.e. it is unlikely that an expression will contain both a [Scalar] and a Matrix<Scalar>.

A major release gives us the unique opportunity to make code-breaking changes.

How

As such I would argue that both APIs can and possibly should be dealt with individually, given their different constraints:

  • [Scalar]: due to dealing with generic collection types public functions can end up polluting the global namespace, adding needless noise to auto-completion, and ambiguity (e.g. Array already uses + for concatenation, forcing Surge to use .+ instead) etc.
  • Vector<Scalar>/Matrix<Scalar>: due to being tied to concrete and Surge-owned types there is no, or at least less, risk of namespace pollution, nor ambiguity.

As mentioned earlier, both APIs also differ in abstractness.

As such I would like to propose the following options going forward:

Option A

  1. Keep all of Surge's API as free functions.
  2. Change functions' naming scheme:
Nonmutating Mutating Idiomatic
Before: z = add(a, b) addInPlace(&a, b) NO
After: z = adding(a, b) add(&a, b) YES

Pro

  • A semi-idiomatic API for [Scalar]/Vector<Scalar>/Matrix<Scalar>.
  • By using @available(*, …, renamed: …) Xcode would be able to provide a reasonably effortless/automated migration path with easy fix-its.

Contra

  • Free functions don't quite feel at home with Swift's dominating OOP syntax.

Option B

  1. Keep Surge's [Scalar] API as free functions, changing their naming scheme to:
Nonmutating Mutating Idiomatic
Before: z = add(a, b) addInPlace(&a, b) NO
After: z = adding(a, b) add(&a, b) YES
  1. Turn Surge's Vector<Scalar>/Matrix<Scalar> APIs into OOP methods, changing their naming scheme to:
Nonmutating Mutating Idiomatic
Before: z = add(a, b) addInPlace(&a, b) NO
After: z = a.adding(b) a.add(b) YES

Pro

  • A semi-idiomatic API for [Scalar].
  • By using @available(*, …, renamed: …) Xcode would be able to provide a reasonably effortless/automated migration path for [Scalar] API changes with easy fix-its.
  • A fully idiomatic API for Vector<Scalar>/Matrix<Scalar>, following the API Design Guidelines.

Contra

  • [Scalar] and Vector<Scalar>/Matrix<Scalar> would have different syntax. However as mentioned earlier it is quite common to have low-level APIs use C-style free functions and high-level APIs use OOP patterns (i.e. methods). Also both APIs as unlikely to get mixed, as Vector<Scalar>/Matrix<Scalar> generally work with instances of Vector<Scalar>, not [Scalar].
  • It seems like @available(*, …, renamed: …) does not provide Xcode fix-its for changing an n-ary free function call into an (n-1)-ary method call on the first argument. We might want to ask the Swift Standard Library team for a recommendation here?

Deprecation vs. Removal

We would also have to decide whether we wanted to mark renamed functions as deprecated, or go straight to unavailable.

@regexident regexident added this to the 3.0 milestone Oct 13, 2019
@regexident regexident self-assigned this Oct 13, 2019
@regexident
Copy link
Collaborator Author

I personally am leaning towards Option B. What are your thoughts, @mattt?

@regexident regexident mentioned this issue Oct 13, 2019
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant