• A guy I knew (unregistered)

    What is also a bit interesting, is the Status property-changed event appears to be raised, but the value... meh, not so much.

  • erichamion (unregistered)

    The comment doesn't even justify making the property private. The justification given supports a public property with a private setter.

  • (nodebb)

    but the value... meh, not so much

    You gotta admit: it does keep clients from changing the status. Mission accomplished!

  • (nodebb)

    I'm a bit confused, because this seems perfectly fine.

    You cannot change the status but there's a setter so that you can get the event that you didn't change the status.

    This is actually how it should be in an API with an observer; you can still trigger the event but you can't change the internal state and the event will appropriately reflect that. There is tons of reasons why you do this, one major one is E2E and integration testing. It's basically design by contract: you can only enforce a contract where a contract can be validated. If the property setter is private, there is no testable contract and this means there will be no tests to enforce it.

  • (nodebb)

    The only personal issue I have with this setter is that it is missing an Obsolete attribute on a warning level.

    BUT I had many clients in the past that used something like Sonar as a management hammer instead of actually for what it is actually intended, as a tool to empower devs to deliver better quality, so I totally understand why developers would avoid proper life cycle management of their APIs, especially when it's an internal one with no consequences. But eh, that's pretty much the only issue I can spot with this code for the reason stated right above.

  • Peter Smith (unregistered)

    Assuming this is C# and it's using the INotifyPropertyChanged interface, there's two problems

    Firstly, C# lets you make the getter public but have a private setter.

    Secondly, the _Status underlying value is never actually set. Presumably the callers have to do a double "_Status = mynewvalue; Status = mynewvalue;" to have anything take effect?

  • (nodebb) in reply to MaxiTB

    I'm a bit confused, because this seems perfectly fine.

    You are, indeed, confused, because you go on to say that this is perfectly fine. In what weird world is it remotely acceptable to (strikeout)have this property even exist(1)(/strikeout) have a property whose "set" method completely ignores the requested new value and simply raises some sort of event that the requested new value was accepted.

    (1) I return to my general thesis that whoever it was that thought that properties were a good idea was ... well, even among first-stripe fucknuts, this person was a first-stripe fucknut.

  • Hmmmm (unregistered)

    They said "private", but meant "read-only." They also added a little nip via the raise, if you try to set the status.

  • Lurk (unregistered)

    RaisePropertyChanged make me suspect WPF and WPF is just... weird; stuff getting updated seven ways from Sunday and it's all behind the green curtain where mere mortals^w cannot venture. I wouldn't be at all surprised to find this the remnant of someone's attempt to work out what the blazes was updating "status" and the public / private confusion is just part of the blind, frustrated, keyboard pounding that was part of it.

  • (nodebb) in reply to Steve_The_Cynic

    I think you approaching that from the wrong assumption that this was new code. We actually don't know that.

    It is totally reasonable to also to see this as a refactoring/bugfix result.

    So basically the code and contract existed before, consumers of the API ignored the convention, a non-native speaker fix the issue and made an appropriate testable change by removing the state change while leaving the test event trigger intact and then wrote a comment the best he could in an foreign language.

    You are judging people based on code without any context way to quickly especially when a simple best-practice explanation is around the corner that perfectly clears up what is going on ;-)

  • (nodebb) in reply to MaxiTB

    I think you approaching that from the wrong assumption that this was new code.

    Absolutely not. There is no excuse(1) for this under any circumstances. If they realised that the transition shouldn't be allowed, they should have (a) published a new version of the API's programming interface, one that has no mention of the previous function, and (b) in a second version, added "this is beyond deprecated and well into WTFF are you doing you fucknut territory" assertion in the offending method.

    Under no circumstances is it acceptable to just swallow a request to change the state, especially if you claim in what appears to be logs that the request was processed.

    (1) Be careful of the difference between "there is a reason" and "there is an excuse". I've never been able to persuade anyone that this difference has any value, but that just means that I've been speaking to idiots.

  • Twither (unregistered) in reply to Steve_The_Cynic

    Everything in the presented snippet is perfectly fine, at least in isolation. Maybe the setter should have simply been private, but if it needs to conform to an existing API, that may not be possible. Raising the PropertyChanged event is not only reasonable, but necessary, especially if this is consumed by WPF. Throwing an exception might make more sense, but again, WPF. A TrySetStatus() method would make more sense, but then data bindings wouldn't work.

    In aggregate, this snippet suggests something is wrong, but not where. Perhaps the API was poorly written, or the component boundaries were poorly designed. From the limited perspective we have, it's really impossible to say where the WTF is.


    I think code like this is the inevitable consequence of having data bindings and polymorphism at the same time. I would propose that TRWTF is polymorphism, not data bindings and properties.

  • (nodebb) in reply to Steve_The_Cynic

    they should have (a) published a new version of the API's programming interface

    That doesn't help.

    In fact, one problem here is that the code fragment is not the definition of an API, it is the implementation. They can't just merrily delete the setter because then it would no longer conform to the API. If clients expect a setter, you can't get rid of it unless you are able to recompile all the clients.

    It seems to me, that somebody decided they no longer wanted the users of an API to be able to change a certain internal variable. They did this by deleting the implementation that allowed the client to change the variable but they had to leave a stub in, other wise all the clients would immediately b real. You don't necessarily have the ability to force everybody to immediately recompile their code against the new version of your API.

    Even the raised event makes some sense. This is some sort of concurrency API, so clearly, clients expect changes to properties by other clients. The coder here has said "I'm going to swallow the attempt to set the property, but the client thinks they've changed it. Therefore, I'll send them the message they would get if some other client changed the property." That allows the client to realign its view of the state of the system with the actual state of the system.

    Addendum 2026-08-13 13:46: s/b real/break/

Leave a comment on “Public Private Partnership”

Log In or post as a guest

Replying to comment #703207:

« Return to Article