kettanaito / naming-cheatsheet

Comprehensive language-agnostic guidelines on variables naming. Home of the A/HC/LC pattern.
MIT License
13.82k stars 861 forks source link

JavaScript getter/setters (S|Get keyword) #49

Open jenbutongit opened 3 years ago

jenbutongit commented 3 years ago

👋 hey, this is a great repo/cheatsheet, thanks for this!

A couple points on your get/set examples, in short, they are confusing when compared with each other and conflicts with OO principles. "Accesses data immediately (i.e. shorthand getter of internal data)."

// Get
function getFruitCount() {
  return this.fruits.length
}

this suggests that getFruitCount is part of either a class or object, which has the property fruits with type array (or other iterable).

const bowl = {
  fruits: ['apple', 'orange', 'pear'],
  getFruitCount() { this.fruits.length },
  get fruitCount() { this.fruits.length }
}

with the example above to get the number of fruits in the bowl, you can do either bowl.getFruitCount() or bowl.fruitCount. If you find you are using getFruitCount often in your code, or you are passing it into another function, it can get unreadable/extraneous quite quickly.

 throw new Error(`Not enough ${bowl.getFruitCount()}`);
 // vs
 throw new Error(`Not enough ${bowl.fruitCount}`);
// Set
let fruits = 0

function setFruits(nextFruits) {
  fruits = nextFruits
}

setFruits(5)

I think this example should either match your get example, or be completely different so it's not confusing, since get/set on an object is thought of to be a matching pair. Currently, I don't see a "benefit" of using this set function over a simple fruits = randomNumber() or fruits = 5 especially with the current scoping. If you were to match the getter method (get keyword), the example would look like this.

const bowl = {
  fruits: 0, 
  get fruitCount() { this.fruits },
  set fruitCount(val) { 
    if (typeof val === 'number') {
      fruits = val
  } else {
     throw new Error('You need to provide a number');
  }
}
console.log(bowl.fruitCount) // 0;
bowl.fruitCount = 5;
console.log(bowl.fruitCount) // 5;
bowl.fruitCount = "invalid"; // throws Error 

the key words, in js and other languages usually allow for those sorts of protections against setting (and getting a private field without exposing the field's value directly).

To avoid this confusion, I think you should either explain getters/setters vs a function with get/set in the name and provide an example of both, or more simply, use a pure function as an example.

function getRandomFruit() { /* returns a fruit */ }
function setTheme(palette) { /* changes UI theme to light | dark mode */ }
turkyden commented 3 years ago

Awesome !

kettanaito commented 2 years ago

Hey, @jenbutongit. Thanks for proposing this!

they are confusing when compared with each other and conflicts with OO principles.

I can't agree with this. The example explicitly features a standalone function, and not a class method, to highlight that it's not bound to OO principles. Getters/setters are specific language features, and while those are also functions, I trust common sense to differentiate between them and plain functions.

I think this example should either match your get example, or be completely different so it's not confusing, since get/set on an object is thought of to be a matching pair.

Agree on this one. The setFruits example is rather abstract and, as you're rightfully pointed out, is pointless on its own. Sometimes it's hard to come up with an example that's both illustrative and meaningful/practical. I understand that the lack of practicality may prevent people from understanding the example thoroughly. I don't mind changing it.

I would still keep it a standalone function, detached from the getters/setters of a class (neither get nor set were meant to address class getters/setters; they just function name prefixes that may bear value outside of simply getting/setting the value, i.e. as getUsers that does async operations).

Currently, I don't see a "benefit" of using this set function over a simple fruits = randomNumber() or fruits = 5

Yeah, once again because you're looking at the function as a practical example while it's value is purely conceptual here. That's a drawback of the current example, we should fix that.

To avoid this confusion, I think you should either explain getters/setters vs a function with get/set in the name and provide an example of both, or more simply, use a pure function as an example.

Your examples are fantastic! 🎉 I would be all hands for changing the existing get/set examples to yours. Would you like to open a pull request with that change, please?

kettanaito commented 2 years ago

Regarding getters/setters, I have really limited knowledge of programming languages, so I'm not confident to say if that's a generic pattern present everywhere or the one that heavily manifests in OO languages like Java. This would define how relevant their inclusion in the guidelines would be to all developers.

I think when you're naming a getter/setter, the Avoid context duplication should help you rather well. If you're declaring a getter, there's no need to name your getter method starting with get—it's a repetition of context (context = getter). That's the philosophy I'd encourage people to apply for getters/setters.

jenbutongit commented 2 years ago

@kettanaito RE: getters/setters, most (probably all) OO languages will have some manifestation of getters and setters. In Java they would just be getThing() although I don’t think they are any different to a class method (not a Java developer) however I think linters will yell at you for not using getter/setter pairs, as well as your peers (it is a well known and adopted pattern).

get/set in swift will work in the same way as JavaScript IIRC.

And agreed on context duplication. I’ll have to have a reread of everything and open up a PR (it’s been a while since I opened this!).

kettanaito commented 2 years ago

No worries! Take your time :) Thank you for looking into this.