Constructor function for persons with hobbies

Question

I'm trying to write a constructor function that generates instances of a user. One of the properties of the user class is an array of the user's hobbies. I'd like my constructor to have a method that generates a string representation of the array of hobbies provided to the constructor function. The goal is to create a grammatically correct sentence containing the hobbies of the Person instance, so if interests = ['hiking', 'biking', 'skiing'] the Person.bio() method will alert something like: This person's interests are: hiking, biking and skiing. I am trying to account for an array of unknown length being passed to the constructor.

I haven't used .reduce() much but from the little bit of testing I've done, this seems to do what I want it to do. I'm just looking for any critiques on how to make things more readable or performant!

Below is a stripped down version of the constructor function (missing things like name, age, etc. for clarity).

function Person(interests = []) {
  this.interests = interests;
  this.hobbiesSentence = interests.reduce((hobbyString, hobby, index, interests) => {
    switch(index) {
      case (interests.length - 1):
        return hobbyString += `${hobby}.`
      case (interests.length - 2):
        return hobbyString += `${hobby} and `
      default:
        return hobbyString += `${hobby}, `
    }
  }, '');

  this.bio = function () {
    alert(`This person's interests are: ${this.hobbiesSentence}`)
  };
}

As I'm writing this, I could see a case for making the function exist but not necessarily creating/storing the sentence unless the bio() method is called. Any other ideas? Thanks in advance.

The word "implementation" is misleading; it made me think this was a reinventing-the-wheel question. Perhaps "usage" would make more sense here? — Alex F
– Alex F, Commented Aug 8, 2019 at 0:14
This question is pretty borderline. In the future, please don't simplify your code for the sake of posting on Code Review. See How to Ask. — 200_success
– 200_success, Commented Aug 8, 2019 at 2:26

ggorlen · Accepted Answer · 2019-08-12 03:37:08Z

Performance

Strings are immutable, so using accumulator += stuffToAppend in a loop can traditionally impact performance. The problem is that we're creating a new string every iteration, leading to quadratic time complexity for an operation that should be linear. It turns out that modern browsers optimize this heavily using an internal array to represent string parts and make it quite fast over using an explicit array, so this post is focused on style rather than performance.

Design

On first thought, reduce seems like the right function from a semantic standpoint since we want to boil the array of interests down to one string. However, since avoiding string concatenation requires an intermediate array in reduce, we might as well just skip the intermediate array and use map and join. It's pretty common that reduce can be replaced with map or filter, which are more specific and succinct.

Switch statements are also generally not used much in JS (but often used in C...). You can replace many switch statements in JS with an object (particularly if you're choosing between a number of similar functions), or at least an if statement. Either way, the nature of the commas and "and" in this example makes it a bit awkward, so there doesn't seem to be any clear-cut win.

Additionally, this routine of "prettifying" a list is generic and can be moved to a separate function to keep Person clean.

As an aside, instead of switching between "interests", "hobbies" and "bio", it seems best to pick one term and stick with it throughout.

Here's my attempt. This might seem a bit abstract, but it's typical in JS to avoid conditional/switch stuff as much as it is to avoid loops (which is the idea with reduce). If you prefer a more traditional approach, replace the joins array and indexing with an if statement and I'd still endorse it.

const prettyList = (a, sep=", ", endSep=[" and ", "."]) => 
  a.map((e, i) => e + (endSep[endSep.length-a.length+i] || sep)).join("")
;

const Person = function (interests=[]) {
  this.interests = interests;
  this.interestsSentence = prettyList(interests);

  this.interestsStr = () => 
    "This person's interests are: " + this.interestsSentence
  ;
};

const interests = ["foo", "bar", "baz", "quux"];

for (let i = 1; i <= 4; i++) {
  console.log(new Person(interests.slice(0, i)).interestsStr());
}

Now the function is reusable and we can change its behavior without much effort at all:

const prettyList = (a, sep=", ", endSep=[" and ", "."]) => 
  a.map((e, i) => e + (endSep[endSep.length-a.length+i] || sep)).join("")
;

const activities = ["biking", "running", "walking", "skipping", "driving"];
console.log("I love", prettyList(activities, "; ", [", sometimes ", " but not ", "!"]));
console.log("I love", prettyList(activities, " and ", [" while ", " :-o"]));

Thanks for taking the time to give some real feedback. As you might expect, I'm pretty green at coding and I posted here looking for constructive ways to write more "real world" JS. OOP is new to me as well, so I wrote this with the idea that everything necessary to modify the "person" instances would be contained on the constructor object. I'll dig through your reply to make sure I understand the nuances. — Anthony
– Anthony, Commented Aug 8, 2019 at 20:38
To me it seems that the string concatenation in your map is the same as it would be in a reduce (or just +). Also, what do you mean by "intermediate array"? — morbusg
– morbusg, Commented Aug 12, 2019 at 1:11
The difference is taking an accumulator string and resultStr += elem versus building an array of elem + someExtraStuff, then joining it for the result. Traditionally, string concatenation is much slower because a totally new string is built on every iteration. Looking into it, it turns out that browsers have optimized it considerably, so it's probably not a major issue as it might be elsewhere. By "intermediate array", I mean that if we want to avoid string concats, we pass [] as the initial accumulator for .reduce and append each element--but wait, that's exactly what map does! — ggorlen
– ggorlen, Commented Aug 12, 2019 at 1:20
Somehow, in my mind I thought: "What is join if not string concatenation", but I don't actually know what happens "under the hood". Also I was thinking giving an empty string as the initial value for the reducer (no need to assign to the accumulator there BTW, since the return value will be passed to the next invocation). But anyway, I agree that the reduce is a bit unwieldy here. I played around with this a little and actually ended up using a switch myself :D — morbusg
– morbusg, Commented Aug 12, 2019 at 2:00
It turns out that modern browsers optimize this heavily using an internal array it seems we no longer need expert developers in this world. Bad code gets optimized anyway :p — dfhwze
– dfhwze, Commented Aug 12, 2019 at 3:51

Stack Exchange Network

Constructor function for persons with hobbies

1 Answer 1

Performance

Design

You must log in to answer this question.

Hot Network Questions

Constructor function for persons with hobbies

1 Answer 1

Performance

Design

You must log in to answer this question.

Related

Hot Network Questions