6 ms·

Getter-Setter Pattern Considered Harmful

• shaftoe444 2y ago
  .
  • alkonaut 2y ago
    The field is final, not it’s contents. It’s not a ”final list” on that field, it’s a (mutable) list that is the final assignment of a field. Mutability or immutability is part of the type not the field so you have 4 combinations in total List<T> list; final List<T> list; final ImmutableList<T> list; ImmutableList<T> list; And that’s not even considering whether each T in the list is mutable or not.
• giancarlostoro 2y ago
  I always thought blank / barebones getters and setters were kind of dumb and genuinely pointless, some obscure ceremonial thing that. C# added syntactic sugar for them. If you are not validating any of the input you might as well just have a public variable.
  • bobmcnamara 2y ago
    I thought there were two reasons. You can add validation for all setters, and abstract it as an interface later if needed.
    • Pxtl 2y ago
      Also lets you maintain your invariants. It's particularly useful for cache invalidation. Like if your object exposes a hash of all its public properties, you can invalidate the cached hash when a prop setter is called with a new value.
      • mrkeen 2y ago
        You just reminded me of one of my main reasons to avoid getters, which the article didn't mention. Accessing a field will not invoke other code.
        • Groxx 2y ago
          Unless it's synchronized. Then you could deadlock.
          • mrkeen 2y ago
            This got me curious, so I tried it out. I was unable to put a synchronized onto the field directly: error: modifier synchronized not allowed here public synchronized final Object field; But when I locked it in code: synchronized (field) { while(true) { .. I was still able to access the synchronized field directly without triggering a deadlock - whereas a getter() could be synchronized and trigger a deadlock. In any case, the article already made a strong points for immutability so you don't need to mess around with deadlocking crap.
            • jayd16 2y ago
              They're saying multiple things can synchronize on the same public field. Same reason you don't want to use this. Although I suppose they could mean the concept and not the keyword. A field might not be thread safe and there's no way for an object to guarantee thread safe access of to a field.
              • mrkeen 2y ago
                Deadlocking was raised as the objection to using public instead of getter. But in regard to deadlocking, using public is strictly better than a getter. You have to take a lock in order to deadlock in the public version. In the getter version, you can take a lock without realising it and then deadlock.
            • Groxx 2y ago
    • tyleo 2y ago
      I’ve written a lot of C# and have heard the proclaimed reasons. I’ve stopped doing this over time. You almost never need the interface and when you do, I haven’t found it hard to add. Maybe you do this by default if you are developing an external API for versioning reasons but that’s such a niche use case.
      • lostmsu 2y ago
        When you change from fields to properties with getters and setters you break binary compatibility with previous versions, which is very bad for libraries.
      • blincoln 2y ago
        How much legacy code have you maintained/added features to vs. writing new code? I find that getters and setters seem like pointless overhead when I have an idea that I want to test empirically as soon as possible, but if I don't, I end up with a lot of regret years later when I have to track down all the references and update them instead of just changing code in the one place. I'm not a fan of lazy loading, but I know a lot of people are, and using the getter/setter pattern also enables fairly straightforward ways to lazy-load properties from persistent storage (or add that capability down the road if it becomes advantageous).
    • almostgotcaught 2y ago
      > abstract it as an interface later if needed this later literally never comes. if you didn't have an ontology when you wrote the code, it never magically unexpectedly appears.
    • elegantlie 2y ago
      I never understood this argument for statically typed languages like Java. > You can add validation for all setters, and abstract it as an interface later if needed. Sure, but why not just postpone the introduction of setters until you actually need them. 90% of time you don't, in which case we just saved a ton of boilerplate and ceremony. The 10% of time that you do, converting public variables to a setter method is about 2 IDE clicks these days. The only argument I can think of is if you are exposing a public library API, in which case you need to worry about backwards API compatibility.
      • jayd16 2y ago
        Generating getters is actually 2 IDE clicks that will protect you down the road. Trying to refactor fields that made it into a public API is a nightmare. If you're shipping code people use, you won't get to refactor their projects from your upstream library. Also, no one is suggesting everything needs a setter. Where is this coming from?
        • alserio 2y ago
          If I could I would make this two IDE clicks behind a big mean warning. In my experience people tends to autopilot it without thinking about what that entails.
• easton 2y ago
  C# has the with part as a language built in; we use it at work with our immutable objects. var y = x with { things I want to change } https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/with-expression https://learn.microsoft.com/en-us/dotnet/csharp/language-ref...
  • jinwoo68 2y ago
    I think C# borrowed this feature from functional languages like Haskell or OCaml.
    • munchler 2y ago
      I would credit F# even more. It's been a main source of new C# features for a while now.
    • antonyt 2y ago
      From F#, most likely! C# converges more and more on F# with every release.
  • SebastianKra 2y ago
    JS/TS has y = { ...x, change: 'b' } And Swift avoids the mutability-problem all together, by providing structs with copy-on-write and self-mutating methods: https://docs.swift.org/swift-book/documentation/the-swift-programming-language/methods/#Modifying-Value-Types-from-Within-Instance-Methods https://docs.swift.org/swift-book/documentation/the-swift-pr...
• zevv 2y ago
  Summary: X is bad. How to do something without X? Don't do X
• samiv 2y ago
  What terrible advice. Your public variables are so much easier to refactor if/when your data changes when you have simple accessors methods around them. Having accessors for data have saved my ass multiple times when I've had to do things such as a) migrate the data transparently b) remove data and use computation instead c) debug reads or writes of data. The last point, sure you can have a breakpoint on a memory address but if you have a getter/setter method its trivial to put a breakpoint in there, but I bet that if you have to set a memory brekapoint you have to waste time googling how do that (at least with GDB) Raw data POD type structs are useful but only when the scope is limited.
  • regularjack 2y ago
    The article doesn't suggest making the properties public, it makes valid suggestions IMHO (though I don't agree with all). I would recommend reading the article.
    • brap 2y ago
      It does this in the end, though
• thecleaner 2y ago
  This is weird. I use getter/setter regularly for DynamoDB backed DTOs. This helps catch setters at the source of a problem before it gets persisted. Also the exceptions will help pinpoint code paths where these things can happen.
• debeloo 2y ago
  Reads like AI garbage.
• x0hm 2y ago
  OOP separates coders from craftsmen.
  • mrkeen 2y ago
    In what sense?
• awfulneutral 2y ago
  This article sort of reads like they think everything should be immutable, which feels kind of dogmatic. Using `with` for everything by default seems like overkill. But in C#, lately I have been using `readonly and `init` wherever vars should not be changed after initialization, which is most of them tbh. For small, data-only objects that can easily be immutable and get passed around a lot, this makes sense to me as a way to avoid the kind of bugs they are talking about.
• tempestrose 2y ago
  This is a waste of time. No One I've ever seen uses getName. It's always name. Also, most of this is useless overkill. If I have, in this case, a person object, I'm doing two things with it. Displaying it. Editing it. If displaying, who gives a crap of its mutable? If I'm editing it, why are you wasting memory making a copy? This just makes life way more difficult in most business apps. Sure, if you NEED this, go for it. But for everyday bog standard business apps? Get the hell away from me.
  • donatj 2y ago
    To me, much of this just gives away a lack of experience. Have you never hit a bug because an object mutated unexpectedly? Please do keep this in mind when you do, and spend hours of your life trying to hunt it down. When you've done this 8 or 9 times after wasting days of your life debugging, you become enlightened to the joy of immutability. It's worth the tiny memory overhead. Beyond that, get* is everywhere in of corporate OO especially in more classical OO languages. I'd argue the right way to do things as well, naming methods as actions, but that's a separate issue.
  • cess11 2y ago
    This was born from needs in the bog where standard business software lives. Sure, you can have naked attributes, but you'll be sorry once they aren't enough and you now have two parallel ideologies seeping through your software, increasing the risk of bugs. Concurrency and collection attributes are the common reasons you'll want getters, possibly also setters.
• renewiltord 2y ago
  Some decades ago as a child I read a Bjarne Stroustrup interview where he said you use those things to enforce invariants and if you don’t you keep access simple. In the intervening decades, I’ve found that when I have a new invariant I often need to confirm the call sites are compliant anyway as he says. So I just use that, keeping accessors only where required or where interface compliance syntactically requires it. https://www.artima.com/articles/the-c-style-sweet-spot https://www.artima.com/articles/the-c-style-sweet-spot
• vips7L 2y ago
  This pattern is really only used with @Entity’s. Things that actually are mutable and rightfully need to be. With @Entity’s setters are exactly what you want because it allows you to add validation, and check error states before persisting. Arguing immutability for the sake of immutability is considered harmful imo. Only make things immutable when they actually are immutable or when concurrency is at play (which it probably isn’t, 99% of all code is single threaded).
  • mrkeen 2y ago
    Why not just prevent the construction of bad states instead of constructing bad states and later checking them?
    • Benjammer 2y ago
      Often, imo, this depends on where you want to put the validation complexity. You can move validation to the front end and "prevent constructing bad states" through a combination of good UI and front end data validation, i.e. validate all the pieces of an entity to make sure the eventual Entity will be valid. But the opposite trade off would be to keep your front end simple, and have the data represent anything that is possible in the UI, and then you can validate anywhere in your stack that you want or where it makes sense for any engineering reason. You can still validate the UI models on the front end before sending them over the wire. You can validate at the endpoint and return HTTP codes. You can run logic in your database to massage/correct/fix the data any way you want. You could even throw things into a queue for post-processing later on, or send to another server for processing, whatever you want. In theory it sounds great to say "just prevent construction of bad states," but this is a complex problem in professional software that involves coordination between programmers, designers, and product managers to get the UI features into the right place where preventing the construction of bad states doesn't lead to a frustrating UI experience for users. It's often far simpler, more pragmatic, and less complex in terms of coupling in both software and humans to just accept whatever state the UI gives you and figure things out somewhere else where you aren't bothering your users so much.
      • 2y ago
• brap 2y ago
  I’m on board with the With and Builder patterns where it makes sense, but the final suggestion to make the fields public is just bad advice. The author recognizes that mutable state gives you a lot to worry about, which I agree with. But public mutable fields make it even worse. Getters and setters at least minimize the surface area and give you some control over mutability. There are many reasons why this is not the common practice, people have learned these lessons before.
• donatj 2y ago
  > Notice the subtle difference? Dropping those get prefixes for the access methods has significant benefits. Let's think about it: What value was added by the prefix? Correct, none! Methods and functions do something rather than are something so naming them as the action they complete seems much better form to me. 3 characters is a fair price for clarity. > Drop the getters and setters. They don't add any value They add the ability for the underlying object to change without breaking compatibility. This is particularly important on shared libraries where you are not going to be able to refactor every single call sight. For instance we have an object in our model representing a building, we've gone through *three* different structures of what a building is, but we've managed to make those changes while maintaining compatibility through leaving deprecated methods that translate the calls.
• geophile 2y ago
  This misses the main problem with get/set methods. In OO othodoxy, you aren't supposed to make state public, because it violates encapsulation. If you want to modify your Point object to use polar coordinates instead of cartesian, then if you expose your cartesian state (x and y), then all the code that relies on x and y is now broken. So what getX() and setX() do is allow your x and y fields to be encapsulated. Problem solved! Except that once you change to polar coordinates you have your choice of problems: 1) Find all the getX() and getY() uses, and rewrite the affected code to use getR() and getTheta(), since you are now using polar coordinates. In other words, you have violated encapsulation by having getX() and getY() methods, and your get/set methodology hasn't achieved anything. 2) Re-implement getX() and getY() in terms of polar coordinates. That's doable. But do you also now have getR() and getTheta()? Why? Having getFoo() and setFoo() methods for each field foo is this dumb idea that began, I think, with JavaBeans. It has nothing to do with OO as I understand it. The idea behind OO encapsulation is to separate your object's behavior (the methods) from private state. You can change state at will as long as you maintain behavior. Mechanically adding behavior (get/set methods) to track your private state has always been a stupid idea that has nothing to do with OO except to violate encapsulation.
  • mlyle 2y ago
    > Mechanically adding behavior (get/set methods) to track your private state has always been a stupid idea that has nothing to do with OO except to violate encapsulation. What should be private state is a matter of debate. If you have a point, it's not too ridiculous to ask "where is it?" Cartesian coordinates may or may not be how the Point object represents where it is, and that internal detail should be hidden from callers. > But do you also now have getR() and getTheta()? Why? It seems like a Point class should hold its internally state in the "best" implementation for how it is usually used, and provide its state out to callers in all of the convenient forms that would be used more than occasionally. Of course, one should avoid purely exposing internal details. And sometimes you might encapsulate more to encourage callers to use the pointA.getDistanceAndBearing(pointB) call. (But I don't think this makes sense for points; knowing their location is too fundamental).
  • ivanbakel 2y ago
    Point 2) feels very weak. You could easily have getR and getTheta, but critically you could also always have had them - if they make sense as APIs, and your state is encapsulated, you can expose them at any time. The benefit of having getters is that API consumers don’t privilege public state above any other API component, because they don’t know what’s actually the real state and what’s just under contract.
• jmull 2y ago
  I don't think this kind of article is that helpful. There are aspects of a valid point in there, but it's buried and confused. That builder pattern makes me sick to my stomach. Also, it starts with claiming state is part of the problem with getters/setters, but does not mention it again. (It seems to be conflating mutability and state, which, while often related in practice, are not necessarily that tightly coupled -- an immutable thing can nevertheless be state, and you can have mutability without introducing state. State is really about things that live beyond the immediate context/scope. So really nothing to do with getter/setter.)
  • mrkeen 2y ago
    Tell me about something which is mutable and non-stateful.
    • jmull 2y ago
      I think you should take a minute to think about it. Let me turn it around on you: Give me a simple example in code of something that is mutable and not state. I'll come back later to check your answer and provide one of my own, if necessary.
      • jmull 2y ago
        OK, here it is: int addOne(int value) { value += 1; return value; } value is clearly mutable, yet not state. State is a value you can refer to later. Here, "value" doesn't outlive the function, so can't be referred to later. State is a complication because you have to worry about whether it remains up-to-date. (Mutable state is even more complex because you also have to worry about if/when it may be changed.)
• deleted 2y ago
  [deleted]
• the_arun 2y ago
  For immutable objects you can use new ‘record’. Another option is to use Lombok which provides an elegant way for reducing code.
• kleiba 2y ago
  Only when you're a Java programmer can you find that "Builder" suggestion attractive.
• recursivedoubts 2y ago
  please for the love of Anders Hejlsberg just add properties to the language already we've had JavaBeans for like a century now we just need properties like any sane language would have added please bro i promise bro i swear just add properties to the langauge so the expression is a valid right hand side and left hand side look at javabeans bro i promise it'll be ok it's not bad to admit that anders was right on this one it'll be ok bro just add properties please bro no more builders just lets add properties bro
• ziofill 2y ago
  I get it, immutability is nice. But doesn’t it come with memory and runtime overheads for having to create and store new objects?
  • feznyng 2y ago
    Depends on the language, some have runtime optimizations to make copies fast (usually FPs with purely immutable data). Not sure if that’s the case in something like Java.
• IgorPartola 2y ago
  I find immutable state to be useful when appropriate. But sometimes you really do want mutability, and it is a feature. Performance of updating player.coordinates, ability to create temporary but potentially complex structures inside the function, lack of need to create a new immutable copy of a bank account’s ledger to add another transaction, all good reasons to allow mutability. It seems that there are some people who really just think of everything as a state machine and want state transitions to happen only in very controlled and specific ways. And other people seem to focus more on how to organize frequently evolutions of mutable state in a way that makes it easy to reason about. I’m not sure if there is some sort of commonality to these approaches. Different parts of the industry maybe? Different exposure to Haskell?
  • mrkeen 2y ago
    > lack of need to create a new immutable copy of a bank account’s ledger to add another transaction, all good reasons to allow mutability. The immutability kool-aid was already drunk by the banking industry 5000 years ago. That's why you're appending transactions on a ledger instead of mutating bank balances (behind getters and setters, so it's "encapsulated" /s).
    • IgorPartola 2y ago
      Sure but your account balance does mutate and so does the length of the ledger. (I know this was sarcasm, but I do think those are important considerations).
      • mrkeen 2y ago
        If your account balance and ledger disagree, which one is right?
• drivingmenuts 2y ago
  is there anything out there that that someone does not consider harmful?
• readthenotes1 2y ago
  I can't count how many bugs I found and fixed by removing setters. I avoided Spring for years because of its asinine favoring of setters over correct construction
• mikenereson 2y ago
  I dont agree with the post, but wait til you hear about `record`.
• joaonmatos 2y ago
  I agree with the article for the most part (except the last snippet where the author reverted to bare, mutable fields), but the point was very badly argued. I think we really need to consider that sometimes you want a place to hold together data, and some other times you are encoding some behavioral trait, and prior to records Java did not have a way of distinguishing between them. My rule of thumb is to avoid exposing data in behavioral classes, and prefer immutable patterns for data holders. But sometimes it's not possible to not have setters (e.g. for builders and for JPA entities).
• Doxin 2y ago
  I feel like this is one of the things python got right. You can start off with plain old data: def Foo: def __init__(self, foo): self.foo=foo Use it in all the ways you like. If then later you discover a need for a getter/setter type setup (because you're now e.g. storing foo in a different format internally) you can switch without any of the code depending on the Foo class needing to change: def Foo: def __init__(self, foo): self.foo=foo @property def foo(self): return unmunge(self._foo) @foo.setter def foo(self, v): self._foo = munge(v) I'll grant that the syntax is slightly wonky, but its really nice to not have to think about getter/setters when initially designing a class. You can add them when and where you need them without breaking other code.
  • Kim_Bruning 2y ago
    def __init__(self, foo): self.foo=foo should, in this example, probably be: def __init__(self, foo): self.foo=munge(foo)
    • Doxin 2y ago
      Ah yeah missed that. Added the munge/unmunge to it as a final pass because I figured it'd clarify the use case slightly better.
• harpiaharpyja 2y ago
  Missed the obvious: age should be replaced with date of birth from which an up-to-date age can always be calculated.