3
\$\begingroup\$

I am quite an experienced developer but I am quite new to JS/frontend/functional programming and I am working with react-native in my day-job now.

I'm trying to do my own flux implementation to better understand redux. My goal is to have a performant data store that will behave predictably. This solution seems to be working for me, but will become slower the longer the app runs.

I've looked at using immutable.js and lodash but I'm not sure it's necessary. I've also considered using a mutable currentState variable and only merging the changes since the last call to getState() and then resetting i.e changes = [] and updating currentState. I'd also like some opinions on my use of a promise in the getState function.

export default class Session {

  static changes = []

  static getState() {
    return new Promise((resolve, reject) => {
      resolve(this.changes.reduce((a, x) => Object.assign(a,x),{}))
    })
  }

  static update(value) {
    this.changes.push(value)
  }

  static showChanges() {
    this.changes.forEach((x)=> console.log(x))
  }
}



Object.defineProperty(Session, 'changes',{
  writable: false,
  enumerable: true
})
\$\endgroup\$
11
  • \$\begingroup\$ Why are you using a promise in getState() when it's a synchronous operation? That will just make it slower than it needs to be and more complicated to use. \$\endgroup\$
    – jfriend00
    Dec 8, 2016 at 6:11
  • \$\begingroup\$ If the reduce function takes too long it will block the UI animations. \$\endgroup\$
    – Cobolt
    Dec 8, 2016 at 6:21
  • \$\begingroup\$ Using the promise doesn't keep .reduce() from blocking anything. .reduce() is synchronous and JS is single thread. The promise could keep whatever code comes next in the caller from blocking the repaint, but it's still really odd to force a synchronous object to use a promise for that purpose. You could probably improve the speed of your .reduce() loop a lot. Also single letter variable names like a and x do not contribute to the readability of your code by someone who doesn't know what you're trying to do. \$\endgroup\$
    – jfriend00
    Dec 8, 2016 at 6:24
  • \$\begingroup\$ I see what you mean. But just removing the promise won't fix the problem then. Do you think this solution would cause blocking if the array becomes too long? Do you think saving a copy of the latest state is a good way around this or would it be better to avoid mutable data? I agree that x and a are bad variable names but it's a convention in F# I got too used to. \$\endgroup\$
    – Cobolt
    Dec 8, 2016 at 6:35
  • 1
    \$\begingroup\$ So, you want the union of all properties in the changes array with the later ones overwriting the older ones? It might be simpler to just maintain that summary as you add each new item to changes. Then, you don't have a time consuming process to build it from scratch when requested. \$\endgroup\$
    – jfriend00
    Dec 8, 2016 at 8:12

1 Answer 1

1
\$\begingroup\$

Interesting question:

  • As per @jfriend00, using a promise is odd there. Your repaint might work out, but in essence JS runs .reduce() single threaded, there really is not a lot of value there

  • Your Session is not immutable, it keeps updating the changes list

  • All your methods are static, I would have expected an "Immutable" class with session as in instance of the Immutable class

  • I would inside your class indeed maintain a mutable object that is up to date ( so a call to update would result in an update of both changes and say an object called state ) Since your class is mutable by having changes, you might as well go all the way.

  • I like mutable.js, in that there is no such thing as getState(), you would get the new state from calling update. This way you can update the data and have the state in 1 line.

All in all, even after reading the whole immutable.js page, I don't see the point. If truly your goal is to have A performant data store that will behave predictably., then a plain old Vanilla Object should do.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.