X-12 / andromeda

0 stars 0 forks source link

Refactor Incorrect Value Checking #75

Open DakotaHarward opened 7 years ago

DakotaHarward commented 7 years ago

We need to refactor how we check for incorrect values submitted via socket.io. Here's an idea:

system.js:

...
validateType(value, expType) {
  if (typeof value != expType) {
    throw createError("wrong type")
  }
}

logError(err) {
  console.error(err.name + " " + err.message)
}

createError(name, message = "") {
  var customError = new Error()
  customError.name = name
  return customError
}

setProtected(ident, value, expType, errMessage = "", customValidation = (value) => {}) {
  try {
    validateType(value, expType)
    customValidation(value)
    set(ident, value)
  } catch(err) {
    err.message = errMessage
    logError(err)
}

some system file:

...
this.setProtected("test", value, "testType", "when setting 'test' in 'some system file'", customValidationForTest)
...
customValidationForTest(value) {
  if(value == "some wrong value")
    this.createError("error name")
  }
}

This is just a very simple semi-reusable error logger. It's not perfect and javascript has a bunch of built in error stuff that we could use, but this is definitely better than what we have now. Thoughts?

jextrevor commented 7 years ago

I really like this idea of integrating error checking into the System function. However, I think this code is a little long-winded. Perhaps something like this would be more clear and concise?

In system.js

set(name,value,type=Null){
    if(type == Null || typeof value == type){
        //normal set stuff
    }
    else{
        console.log("Invalid type "+typeof value+", expected "+type)
    }
}
DakotaHarward commented 7 years ago

Ok, I see what you mean by making the code more concise, and I like the idea of just making the set function equivalent to the setProtected function, but we lose some functionality if we do it the way you specified. Do you like the workflow of my idea? It can be rewritten when we implement this fix.

jextrevor commented 7 years ago

Yeah - I think we should just write the error checking into the set function itself instead of making a new function setProtected which will be longer to type. We can also simplify things. I say go ahead with this one Dakota

jextrevor commented 7 years ago

Or, @DakotaHarward, would you like one of us to do it?

jextrevor commented 7 years ago

What should we do for value checking for things that don't fall within the "setting" category? Something where the values have to be processed before setting?

DakotaHarward commented 7 years ago

We'll just put the checking in a separate function