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

CTL-757: Support CallerMemberName for platforms that support it #749

Closed
GeertvanHorrik opened this Issue Oct 20, 2015 · 10 comments

Comments

Projects
None yet
3 participants
@GeertvanHorrik
Member

GeertvanHorrik commented Oct 20, 2015

Jira issue originally created by user @GeertvanHorrik:

Something like this:

protected internal void SetValue(
#if !NET40
[CallerMemberName]
#endif
string name, object value)
        {
            SetValue(name, value, true, true);
        }

Current locations:

  • ModelBase.SetValue
  • ModelBase.GetValue
  • ObservableObject.RaisePropertyChanged
@GeertvanHorrik

This comment has been minimized.

Show comment
Hide comment
@GeertvanHorrik

GeertvanHorrik Oct 20, 2015

Member

Comment created by @GeertvanHorrik:

Research if using CallerMemberName is possible inside extension methods.

Member

GeertvanHorrik commented Oct 20, 2015

Comment created by @GeertvanHorrik:

Research if using CallerMemberName is possible inside extension methods.

@GeertvanHorrik GeertvanHorrik added the task label Apr 12, 2017

@GeertvanHorrik GeertvanHorrik added this to the Up for grabs milestone Apr 12, 2017

@GeertvanHorrik GeertvanHorrik self-assigned this Apr 12, 2017

@GeertvanHorrik

This comment has been minimized.

Show comment
Hide comment
@GeertvanHorrik

GeertvanHorrik Jul 24, 2017

Member

We dropped support for SL & .NET 4.0. This is probably now supported on all platforms, so we might need to look into this. @alexfdezsauco , can you investigate?

Member

GeertvanHorrik commented Jul 24, 2017

We dropped support for SL & .NET 4.0. This is probably now supported on all platforms, so we might need to look into this. @alexfdezsauco , can you investigate?

@GeertvanHorrik GeertvanHorrik modified the milestones: 5.1.0, Up for grabs Jul 24, 2017

@GeertvanHorrik

This comment has been minimized.

Show comment
Hide comment
@GeertvanHorrik

GeertvanHorrik Jul 24, 2017

Member

And make sure to double-check whether this results in a breaking change or not. If so, this will be milestone 6.0

Member

GeertvanHorrik commented Jul 24, 2017

And make sure to double-check whether this results in a breaking change or not. If so, this will be milestone 6.0

@alexfdezsauco

This comment has been minimized.

Show comment
Hide comment
@alexfdezsauco

alexfdezsauco Jul 26, 2017

Member
Member

alexfdezsauco commented Jul 26, 2017

@alexfdezsauco

This comment has been minimized.

Show comment
Hide comment
@alexfdezsauco

alexfdezsauco Jul 27, 2017

Member
Member

alexfdezsauco commented Jul 27, 2017

@GeertvanHorrik

This comment has been minimized.

Show comment
Hide comment
@GeertvanHorrik

GeertvanHorrik Jul 27, 2017

Member

I am not sure whether I like the Set and Get (which derives from our GetValue and SetValue). Let me think about this one.

Member

GeertvanHorrik commented Jul 27, 2017

I am not sure whether I like the Set and Get (which derives from our GetValue and SetValue). Let me think about this one.

@DanielKuehner

This comment has been minimized.

Show comment
Hide comment
@DanielKuehner

DanielKuehner Aug 16, 2017

Contributor

I came across this issue because a have already implemented the [CallerMemberName] attribute and wanted to switch to your ObservableObject but there I have missed it.

So I took a few minutes to transform your class and add [CallerMemberName] but I failed because of:

  • A normal parameter with [CallerMemberName] becomes a optional parameter
  • Thus moving the porpertName parameter to the end of the parameter list conflicts with existing overloads

In my opinion this might be a big breaking change across Catel and all using codes. Because on the one hand you could introduce other named methods or on the other hand you have to change the call stack because of rearranged parameters and additional methods to avoid same signatures because of object parameters.

Simple example:
protected internal void RaisePropertyChanged(string propertyName, object newValue)
gets
protected internal void RaisePropertyChanged(object newValue, [CallerMemberName] string propertyName = "")
but conflicts with
protected internal void RaisePropertyChanged(object sender, string propertyName)

But how to resolve this? Maybe we have to add all parameters like newValue and oldValue to the RaisePropertyChanged methods and everything is easy :-)

Contributor

DanielKuehner commented Aug 16, 2017

I came across this issue because a have already implemented the [CallerMemberName] attribute and wanted to switch to your ObservableObject but there I have missed it.

So I took a few minutes to transform your class and add [CallerMemberName] but I failed because of:

  • A normal parameter with [CallerMemberName] becomes a optional parameter
  • Thus moving the porpertName parameter to the end of the parameter list conflicts with existing overloads

In my opinion this might be a big breaking change across Catel and all using codes. Because on the one hand you could introduce other named methods or on the other hand you have to change the call stack because of rearranged parameters and additional methods to avoid same signatures because of object parameters.

Simple example:
protected internal void RaisePropertyChanged(string propertyName, object newValue)
gets
protected internal void RaisePropertyChanged(object newValue, [CallerMemberName] string propertyName = "")
but conflicts with
protected internal void RaisePropertyChanged(object sender, string propertyName)

But how to resolve this? Maybe we have to add all parameters like newValue and oldValue to the RaisePropertyChanged methods and everything is easy :-)

@GeertvanHorrik

This comment has been minimized.

Show comment
Hide comment
@GeertvanHorrik

GeertvanHorrik Aug 16, 2017

Member

Yeah, that's why I'm not very confident in applying this code. For the ModelBase, it's not a big deal (we have Catel.Fody for that), but for ObservableObject, it gets a bit more complex. I need to think about this, but definitely not in 5.1 (we want to finalize that asap).

Member

GeertvanHorrik commented Aug 16, 2017

Yeah, that's why I'm not very confident in applying this code. For the ModelBase, it's not a big deal (we have Catel.Fody for that), but for ObservableObject, it gets a bit more complex. I need to think about this, but definitely not in 5.1 (we want to finalize that asap).

@GeertvanHorrik

This comment has been minimized.

Show comment
Hide comment
@GeertvanHorrik

GeertvanHorrik Mar 22, 2018

Member

@alexfdezsauco is still still something we can introduce as a non-breaking change or shall we just close this ticket ?

Member

GeertvanHorrik commented Mar 22, 2018

@alexfdezsauco is still still something we can introduce as a non-breaking change or shall we just close this ticket ?

@GeertvanHorrik GeertvanHorrik modified the milestones: 5.4.0, 6.0.0 Apr 12, 2018

@stale

This comment has been minimized.

Show comment
Hide comment
@stale

stale bot Jun 11, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale bot commented Jun 11, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jun 11, 2018

@stale stale bot closed this Jun 18, 2018

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