Skip to content

Instantly share code, notes, and snippets.

@dwineman
Last active January 8, 2016 12:02
Show Gist options
  • Star 2 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save dwineman/d6c56ec0c0e2fdb761db to your computer and use it in GitHub Desktop.
Save dwineman/d6c56ec0c0e2fdb761db to your computer and use it in GitHub Desktop.
Swift allows you to call instance methods from other instance methods with no scope prefix, indistinguishably from function calls. This leads to the dangerous situation of function calls being shadowed when identically-named instance methods are added to a base class.
// Derived class contains an instance method that calls a function foo() in its scope.
class Base {}
func foo() -> String {
return "function foo()"
}
class Derived: Base {
func bar() -> String {
return foo()
}
}
var f = Derived()
f.bar() // returns "function foo()"
// But later, perhaps in a subsequent framework version or a class extension, the base class
// is updated by adding an instance method also called foo().
// The behavior of the derived class now changes, because method foo() shadows function foo():
class Base {
func foo() -> String {
return "method Base.foo()"
}
}
f.bar() // returns "method Base.foo()"
// Unfortunately, you can't opt out of the danger by choosing not to use this feature. Any
// function call in a class with one or more superclasses or extensions you don't control
// is at risk.
@owensd
Copy link

owensd commented Jul 3, 2014

Nice catch... that's terrible. Do you have a radar logged for this?

@drumnkyle
Copy link

I'm not seeing what's so terrible. That makes sense to me. First of all, you shouldn't be referencing a method outside of a class from within a class. And there should always be preference to a class's method.

@jonpryor
Copy link

jonpryor commented Jul 4, 2014

"Kinda bad," perhaps, but also identical to C++, which has been used with this behavior for going on 30 years...

@dwineman
Copy link
Author

dwineman commented Jul 4, 2014

@owensd: Working on it.

@drumnkyle: It's a function, not a method, but there's nothing wrong with calling functions in this context.

@jonpryor: It's a bad idea in C++ too. C++ is a terrible language to emulate if your priorities include safety and correctness. (Also, I believe C++ provides ::foo() as a way of scoping the call to the free function whether it's been shadowed or not. Swift doesn't seem to have an equivalent syntax.)

@rnapier
Copy link

rnapier commented Jul 27, 2014

C++ does not have class extensions. As a C++ programmer, I've run into the confusion this feature causes, but in Swift it can inject spooky-at-a-distance bugs that are less likely in C++. The fact that C++ developers have grown used to it is not good evidence for it's safety or appropriateness in Swift. Apple encourages developers to use extensions to organize code, and this exacerbates the problem.

For some more examples of how this plays out (and in some cases, dependent on code ordering, and in even due to code in completely different files), see:

https://gist.github.com/rnapier/478465d1b15e95b98b42
https://gist.github.com/rnapier/4213dc64206b17df6935

These can all be addressed just by requiring "self." when accessing methods and properties rather than making it optional (as has been done in ObjC for decades). Implicitness in this case is too ambiguous to the compiler.

@ilyannn
Copy link

ilyannn commented Dec 13, 2015

This is a very good example of the fragile code, but I don't agree that it should be solved by changing instance name semantics.

Note how the code currently pollutes module namespace with a global function foo(). That's never a good idea, so resolving this problem, either by creating a suitable name for a public function or by hiding the implementation detail inside the class, immediately solves the original one:

class Base {}

public func DerivedBarImplementerFooStep() -> String {
  return "function foo()"
}

class Derived: Base {
  func bar() -> String {
    return DerivedBarImplementerFooStep()
  }
}

or

class Base {}

class Derived: Base {

    private static func foo() -> String {
      return "function foo()"
    }

  func bar() -> String {
    return foo()
  }
}

The only way to even have a chance of creating this problem is by using a pattern where private helper functions are used inside a short implementation file and are given some throwaway names.

Still, this is a useful pattern and there's no reason to stop using it; just note that those functions are technically globals and so their names should start with an uppercase letter according to every Swift style guide I read:

private func Foo() -> String {
  return "function foo()"
}

class Base {}

class Derived: Base {

  func bar() -> String {
    return Foo()  // Voila – no ambiguity!
  } 
}

@dwineman
Copy link
Author

dwineman commented Jan 8, 2016

Well, I guess that settles that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment