[Pitch] mutable `newValue` in willSet


(Karl) #1

Sometimes you would like to modify the value being set to a property before it is set.

Currently, you would have to add a separate backing property and implement the getter and setter, even if you want to perform a simple bounds-check on the value being set and want to cap it to allowed values.

e.g:

var bounds : Bounds {
    willSet {
      // Cap the bounds
      newValue = newValue.capped(to: maximumSize)
      // Some comments to demonstrate noise if we were doing more processing
      // Some comments to demonstrate noise if we were doing more processing
      // Some comments to demonstrate noise if we were doing more processing
    }

    didSet {
      // Load the new bounds
      // Some comments to demonstrate noise if we were doing more processing
      // Some comments to demonstrate noise if we were doing more processing
      // Some comments to demonstrate noise if we were doing more processing
    }
}

Against the workaround you have to do currently:

var _bounds : Bounds
var bounds : Bounds {
    get { return _bounds }
    set {
      // Cap the bounds
      _bounds = newValue.capped(to: maximumSize)
      // Some comments to demonstrate noise if we were doing more processing
      // Some comments to demonstrate noise if we were doing more processing
      // Some comments to demonstrate noise if we were doing more processing

      // Load the new bounds
      // Some comments to demonstrate noise if we were doing more processing
      // Some comments to demonstrate noise if we were doing more processing
      // Some comments to demonstrate noise if we were doing more processing
    }
}

Currently, the capping in willSet is a compile-time error because `newValue` is a let constant, but I find that breaking your accessor code up in to willSet/didSet blocks allows for greater readability, especially when you have lots of processing to do (in the workaround example, the validation/preprocessing code and effects/postprocessing code are only separated by a comment). I propose that, at least for the scope of willSet (and *not* didSet, if we can manage that), that the variable should be mutable.

Any thoughts?

Karl


(Xiaodi Wu) #2

I've run up against this too and had a similar thought. But on reflection I
think the current "workaround" is actually the superior solution.

Even though in your example it's very clear that newValue is being mutated,
if newValue were mutable, it would be possible to *accidentally* mutate it
without warning or error if you are calling certain methods on newValue
instead of using an assignment operator. For instance, in other contexts,
I've made such a mistake more than once with array.append() when array is
mutable, and I can never quite remember which of popFirst, dropFirst, etc.,
is a non-mutating term-of-art and which is mutating.

Of course, this can happen with any mutable value, but in all other
circumstances you actually write "var something", whereas in your proposal
you never have to write "var newValue". In fact, what you're actually
proposing isn't even the equivalent of "var newValue"; it's to have
newValue be of type `inout T` instead of `T`. I think such an implicit
inout has no precedent in Swift and would be confusing to users.

By contrast, I think the current solution is very clear and hard to make a
mistake with, although it is a little wordy.

···

On Sun, Sep 11, 2016 at 10:50 PM Karl via swift-evolution < swift-evolution@swift.org> wrote:

Sometimes you would like to modify the value being set to a property
before it is set.

Currently, you would have to add a separate backing property and implement
the getter and setter, even if you want to perform a simple bounds-check on
the value being set and want to cap it to allowed values.

e.g:

var bounds : Bounds {
    willSet {
      // Cap the bounds
      newValue = newValue.capped(to: maximumSize)
      // Some comments to demonstrate noise if we were doing more
processing
      // Some comments to demonstrate noise if we were doing more
processing
      // Some comments to demonstrate noise if we were doing more
processing
    }

    didSet {
      // Load the new bounds
      // Some comments to demonstrate noise if we were doing more
processing
      // Some comments to demonstrate noise if we were doing more
processing
      // Some comments to demonstrate noise if we were doing more
processing
    }
}

Against the workaround you have to do currently:

var _bounds : Bounds
var bounds : Bounds {
    get { return _bounds }
    set {
      // Cap the bounds
      _bounds = newValue.capped(to: maximumSize)
      // Some comments to demonstrate noise if we were doing more
processing
      // Some comments to demonstrate noise if we were doing more
processing
      // Some comments to demonstrate noise if we were doing more
processing

      // Load the new bounds
      // Some comments to demonstrate noise if we were doing more
processing
      // Some comments to demonstrate noise if we were doing more
processing
      // Some comments to demonstrate noise if we were doing more
processing
    }
}

Currently, the capping in willSet is a compile-time error because
`newValue` is a let constant, but I find that breaking your accessor code
up in to willSet/didSet blocks allows for greater readability, especially
when you have lots of processing to do (in the workaround example, the
validation/preprocessing code and effects/postprocessing code are only
separated by a comment). I propose that, at least for the scope of willSet
(and *not* didSet, if we can manage that), that the variable should be
mutable.

Any thoughts?

Karl

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


(Charlie Monroe) #3

The current solution is IMHO fine if you implement the property. Once you're overriding it, I don't think it's pretty that you need to override the getter as well:

override var bounds: CGRect {
  get {
    return super.bounds
  }
  set {
    var bounds = newValue
    // modify bounds
    super.bounds = bounds
  }
}

I'd suggest requiring "mutating willSet" if you want to modify the value:

override var bounds: CGRect {
  mutating willSet {
    /// modify newValue
  }
}

Given the required "mutating" keyword, you can't make a mistake by accidently modifying the value.

···

On Sep 12, 2016, at 8:03 AM, Xiaodi Wu via swift-evolution <swift-evolution@swift.org> wrote:

I've run up against this too and had a similar thought. But on reflection I think the current "workaround" is actually the superior solution.

Even though in your example it's very clear that newValue is being mutated, if newValue were mutable, it would be possible to *accidentally* mutate it without warning or error if you are calling certain methods on newValue instead of using an assignment operator. For instance, in other contexts, I've made such a mistake more than once with array.append() when array is mutable, and I can never quite remember which of popFirst, dropFirst, etc., is a non-mutating term-of-art and which is mutating.

Of course, this can happen with any mutable value, but in all other circumstances you actually write "var something", whereas in your proposal you never have to write "var newValue". In fact, what you're actually proposing isn't even the equivalent of "var newValue"; it's to have newValue be of type `inout T` instead of `T`. I think such an implicit inout has no precedent in Swift and would be confusing to users.

By contrast, I think the current solution is very clear and hard to make a mistake with, although it is a little wordy.

On Sun, Sep 11, 2016 at 10:50 PM Karl via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
Sometimes you would like to modify the value being set to a property before it is set.

Currently, you would have to add a separate backing property and implement the getter and setter, even if you want to perform a simple bounds-check on the value being set and want to cap it to allowed values.

e.g:

var bounds : Bounds {
    willSet {
      // Cap the bounds
      newValue = newValue.capped(to: maximumSize)
      // Some comments to demonstrate noise if we were doing more processing
      // Some comments to demonstrate noise if we were doing more processing
      // Some comments to demonstrate noise if we were doing more processing
    }

    didSet {
      // Load the new bounds
      // Some comments to demonstrate noise if we were doing more processing
      // Some comments to demonstrate noise if we were doing more processing
      // Some comments to demonstrate noise if we were doing more processing
    }
}

Against the workaround you have to do currently:

var _bounds : Bounds
var bounds : Bounds {
    get { return _bounds }
    set {
      // Cap the bounds
      _bounds = newValue.capped(to: maximumSize)
      // Some comments to demonstrate noise if we were doing more processing
      // Some comments to demonstrate noise if we were doing more processing
      // Some comments to demonstrate noise if we were doing more processing

      // Load the new bounds
      // Some comments to demonstrate noise if we were doing more processing
      // Some comments to demonstrate noise if we were doing more processing
      // Some comments to demonstrate noise if we were doing more processing
    }
}

Currently, the capping in willSet is a compile-time error because `newValue` is a let constant, but I find that breaking your accessor code up in to willSet/didSet blocks allows for greater readability, especially when you have lots of processing to do (in the workaround example, the validation/preprocessing code and effects/postprocessing code are only separated by a comment). I propose that, at least for the scope of willSet (and *not* didSet, if we can manage that), that the variable should be mutable.

Any thoughts?

Karl

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Karl) #4

Okay, it’s a fair point that it should be explicit. As I see it there are a couple of options:

1. “mutating willSet”

This is very clean, my only concern is that “mutating” typically refers to the containing type. OTOH, all setters are mutating (aren’t they?), so it should be simple enough to disambiguate.

2. “willSet(var newValue)”

Using the existing implicit variable name override mechanism, we could define the parameter as variable. The downside is that we recently removed this syntax from function parameters, so we possibly shouldn’t reintroduce something so similar. OTOH, the reasoning behind removing that syntax from function parameters (that you can define a variable in the function body which serves the same purpose) doesn’t apply in this case.

3. Returning a value from willSet

Make it so that you may optionally return a substitute value in willSet (which must be the same type as the property). When the compiler sees such a willSet implementation, it will synthesise an appropriate setter. I like this from a mutability perspective, but there may be a performance cost for large data-types and we don’t have this notion of a non-required return anywhere else in the language.

I would be happy with any of them, to be honest.

Karl

···

On 12 Sep 2016, at 08:37, Charlie Monroe <charlie@charliemonroe.net> wrote:

The current solution is IMHO fine if you implement the property. Once you're overriding it, I don't think it's pretty that you need to override the getter as well:

override var bounds: CGRect {
  get {
    return super.bounds
  }
  set {
    var bounds = newValue
    // modify bounds
    super.bounds = bounds
  }
}

I'd suggest requiring "mutating willSet" if you want to modify the value:

override var bounds: CGRect {
  mutating willSet {
    /// modify newValue
  }
}

Given the required "mutating" keyword, you can't make a mistake by accidently modifying the value.

On Sep 12, 2016, at 8:03 AM, Xiaodi Wu via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

I've run up against this too and had a similar thought. But on reflection I think the current "workaround" is actually the superior solution.

Even though in your example it's very clear that newValue is being mutated, if newValue were mutable, it would be possible to *accidentally* mutate it without warning or error if you are calling certain methods on newValue instead of using an assignment operator. For instance, in other contexts, I've made such a mistake more than once with array.append() when array is mutable, and I can never quite remember which of popFirst, dropFirst, etc., is a non-mutating term-of-art and which is mutating.

Of course, this can happen with any mutable value, but in all other circumstances you actually write "var something", whereas in your proposal you never have to write "var newValue". In fact, what you're actually proposing isn't even the equivalent of "var newValue"; it's to have newValue be of type `inout T` instead of `T`. I think such an implicit inout has no precedent in Swift and would be confusing to users.

By contrast, I think the current solution is very clear and hard to make a mistake with, although it is a little wordy.

On Sun, Sep 11, 2016 at 10:50 PM Karl via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
Sometimes you would like to modify the value being set to a property before it is set.

Currently, you would have to add a separate backing property and implement the getter and setter, even if you want to perform a simple bounds-check on the value being set and want to cap it to allowed values.

e.g:

var bounds : Bounds {
    willSet {
      // Cap the bounds
      newValue = newValue.capped(to: maximumSize)
      // Some comments to demonstrate noise if we were doing more processing
      // Some comments to demonstrate noise if we were doing more processing
      // Some comments to demonstrate noise if we were doing more processing
    }

    didSet {
      // Load the new bounds
      // Some comments to demonstrate noise if we were doing more processing
      // Some comments to demonstrate noise if we were doing more processing
      // Some comments to demonstrate noise if we were doing more processing
    }
}

Against the workaround you have to do currently:

var _bounds : Bounds
var bounds : Bounds {
    get { return _bounds }
    set {
      // Cap the bounds
      _bounds = newValue.capped(to: maximumSize)
      // Some comments to demonstrate noise if we were doing more processing
      // Some comments to demonstrate noise if we were doing more processing
      // Some comments to demonstrate noise if we were doing more processing

      // Load the new bounds
      // Some comments to demonstrate noise if we were doing more processing
      // Some comments to demonstrate noise if we were doing more processing
      // Some comments to demonstrate noise if we were doing more processing
    }
}

Currently, the capping in willSet is a compile-time error because `newValue` is a let constant, but I find that breaking your accessor code up in to willSet/didSet blocks allows for greater readability, especially when you have lots of processing to do (in the workaround example, the validation/preprocessing code and effects/postprocessing code are only separated by a comment). I propose that, at least for the scope of willSet (and *not* didSet, if we can manage that), that the variable should be mutable.

Any thoughts?

Karl

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution


(Xiaodi Wu) #5

I have the same concern about "mutating" as you do regarding inconsistency
with the rest of the language.

As I mentioned, "var newValue" is incorrect. One reason it was removed is
that people kept trying to use it when they meant inout, which is what you
are doing here. You would need to write "willSet(newValue: inout T)". "var
newValue" would shadow the variable that you want to change.

Returning a value has problems as you described and is inconsistent with
the rest of the language.

At this point, I'd prefer the status quo over any of these.

One alternative you haven't mentioned is setting the variable directly from
willSet ("bounds = newValue + 1"). The rules can be changed such that this
is allowed and does not trigger a new willSet and didSet (much like setting
the property inside init does not).

···

On Mon, Sep 12, 2016 at 07:32 Karl <razielim@gmail.com> wrote:

Okay, it’s a fair point that it should be explicit. As I see it there are
a couple of options:

1. “mutating willSet”

This is very clean, my only concern is that “mutating” typically refers to
the containing type. OTOH, all setters are mutating (aren’t they?), so it
should be simple enough to disambiguate.

2. “willSet(var newValue)”

Using the existing implicit variable name override mechanism, we could
define the parameter as variable. The downside is that we recently removed
this syntax from function parameters, so we possibly shouldn’t reintroduce
something so similar. OTOH, the reasoning behind removing that syntax from
function parameters (that you can define a variable in the function body
which serves the same purpose) doesn’t apply in this case.

3. Returning a value from willSet

Make it so that you may optionally return a substitute value in willSet
(which must be the same type as the property). When the compiler sees such
a willSet implementation, it will synthesise an appropriate setter. I like
this from a mutability perspective, but there may be a performance cost for
large data-types and we don’t have this notion of a non-required return
anywhere else in the language.

I would be happy with any of them, to be honest.

Karl

On 12 Sep 2016, at 08:37, Charlie Monroe <charlie@charliemonroe.net> > wrote:

The current solution is IMHO fine if you implement the property. Once
you're overriding it, I don't think it's pretty that you need to override
the getter as well:

override var bounds: CGRect {
get {
return super.bounds
}
set {
var bounds = newValue
// modify bounds
super.bounds = bounds
}
}

I'd suggest requiring "mutating willSet" if you want to modify the value:

override var bounds: CGRect {
mutating willSet {
/// modify newValue
}
}

Given the required "mutating" keyword, you can't make a mistake by
accidently modifying the value.

On Sep 12, 2016, at 8:03 AM, Xiaodi Wu via swift-evolution < > swift-evolution@swift.org> wrote:

I've run up against this too and had a similar thought. But on reflection
I think the current "workaround" is actually the superior solution.

Even though in your example it's very clear that newValue is being
mutated, if newValue were mutable, it would be possible to *accidentally*
mutate it without warning or error if you are calling certain methods on
newValue instead of using an assignment operator. For instance, in other
contexts, I've made such a mistake more than once with array.append() when
array is mutable, and I can never quite remember which of popFirst,
dropFirst, etc., is a non-mutating term-of-art and which is mutating.

Of course, this can happen with any mutable value, but in all other
circumstances you actually write "var something", whereas in your proposal
you never have to write "var newValue". In fact, what you're actually
proposing isn't even the equivalent of "var newValue"; it's to have
newValue be of type `inout T` instead of `T`. I think such an implicit
inout has no precedent in Swift and would be confusing to users.

By contrast, I think the current solution is very clear and hard to make a
mistake with, although it is a little wordy.

On Sun, Sep 11, 2016 at 10:50 PM Karl via swift-evolution < > swift-evolution@swift.org> wrote:

Sometimes you would like to modify the value being set to a property
before it is set.

Currently, you would have to add a separate backing property and
implement the getter and setter, even if you want to perform a simple
bounds-check on the value being set and want to cap it to allowed values.

e.g:

var bounds : Bounds {
    willSet {
      // Cap the bounds
      newValue = newValue.capped(to: maximumSize)
      // Some comments to demonstrate noise if we were doing more
processing
      // Some comments to demonstrate noise if we were doing more
processing
      // Some comments to demonstrate noise if we were doing more
processing
    }

    didSet {
      // Load the new bounds
      // Some comments to demonstrate noise if we were doing more
processing
      // Some comments to demonstrate noise if we were doing more
processing
      // Some comments to demonstrate noise if we were doing more
processing
    }
}

Against the workaround you have to do currently:

var _bounds : Bounds
var bounds : Bounds {
    get { return _bounds }
    set {
      // Cap the bounds
      _bounds = newValue.capped(to: maximumSize)
      // Some comments to demonstrate noise if we were doing more
processing
      // Some comments to demonstrate noise if we were doing more
processing
      // Some comments to demonstrate noise if we were doing more
processing

      // Load the new bounds
      // Some comments to demonstrate noise if we were doing more
processing
      // Some comments to demonstrate noise if we were doing more
processing
      // Some comments to demonstrate noise if we were doing more
processing
    }
}

Currently, the capping in willSet is a compile-time error because
`newValue` is a let constant, but I find that breaking your accessor code
up in to willSet/didSet blocks allows for greater readability, especially
when you have lots of processing to do (in the workaround example, the
validation/preprocessing code and effects/postprocessing code are only
separated by a comment). I propose that, at least for the scope of willSet
(and *not* didSet, if we can manage that), that the variable should be
mutable.

Any thoughts?

Karl

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

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