0

I have caluclateAcess function which takes an array as input. The array will have max of three elements and min of one element. So the possible elements it can come is create, read and update. So it's basically 2^3 which means 8 probabilities can come so I am listing down all the probabilities and returning a value.

I will list down the possible input and what output should I return. Array of empty won't be coming which means false, false, false.

create => 'CreateAndRead'
read => 'Read'
update => 'UpdateAndRead'
create,read => 'CreateAndRead'
update, read => 'UpdateAndRead'
create, update => 'ALL'
create,read, update => 'ALL'

So I have written the below function; is there any better way to achieve this?

   let READ = 'read';
   let CREATE = 'create';
   let UPDATE = 'update';


   const caluclateAcess = (inputArray) => {
  if (
    (inputArray.indexOf(READ) > -1
    && inputArray.indexOf(UPDATE) > -1
    && inputArray.indexOf(CREATE) > -1)
    || 
    (
      inputArray.indexOf(UPDATE) > -1
      && inputArray.indexOf(CREATE) > -1
    )
  ) {
    return 'ALL';
  }
  if (
    (inputArray.indexOf(CREATE) > -1
    && inputArray.indexOf(READ) > -1)
    || (inputArray.indexOf(CREATE) > -1
    && (inputArray.indexOf(READ) === -1 && inputArray.indexOf(UPDATE) === -1))
  ) {
    return 'CreateAndRead';
  }
  if (
    (inputArray.indexOf(UPDATE) > -1
    && inputArray.indexOf(READ) > -1)
    || (inputArray.indexOf(UPDATE) > -1
    && (inputArray.indexOf(READ) === -1 && inputArray.indexOf(CREATE) === -1))
  ) {
    return 'UpdateAndRead';
  }
  if (inputArray.indexOf(READ) > -1) {
    return 'Read';
  }
};
2
  • Uh...check what is available once only, instead of iterating the array multiple times, for a start. Commented Jul 3, 2019 at 10:25
  • but it should match any of the condition isn't, may be i am confused Commented Jul 3, 2019 at 10:30

1 Answer 1

1

You can start off by removing the repeat inputArray.indexOf() calls - it makes everything harder to read. It's simpler to check these once only:

const hasCreate = inputArray.indexOf(CREATE) > -1;
const hasUpdate = inputArray.indexOf(UPDATE) > -1;
const hasRead = inputArray.indexOf(READ) > -1;

Second, the rules you set up show that you have three access properties - read, update, and create, each one may imply another. Looking at those, it becomes clear that you have the following relationships:

read => read
update => update, read
create => create, read

Thus, update and create are actually compound. This means that the initial check can be modified to account for these:

const hasCreate = inputArray.indexOf(CREATE) > -1;
const hasUpdate = inputArray.indexOf(UPDATE) > -1;
const hasRead = hasCreate || hasUpdate || inputArray.indexOf(READ) > -1;

This avoids having the checks for if somebody has read or update.

As an extra note, you can just use Array#includes instead of checking the index.

With that said, the read right is pretty much inconsequential. It matters if it's the only one available, in all other cases it's pretty much ignored or assumed to be present. Still, it's useful to model the implied rules - perhaps this can change in the future.

Finally, the logic is too complex. There are only four possible final states and as mentioned above, read is not even used for most of them. Here is the updated code that does all the checks. I've added another state called "None" for the case when there are no permissions at all. Even if it's not possible for this to happen, I find it easier to just have it and it be unused than omit it:

const caluclateAcess = (inputArray) => {
  const hasCreate = inputArray.includes('create');
  const hasUpdate = inputArray.includes('update');
  const hasRead = hasCreate || hasUpdate || inputArray.includes('read');
  
  if (hasCreate === true && hasUpdate === true) return "ALL";
  if (hasCreate) return "CreateAndRead";
  if (hasUpdate) return "UpdateAndRead";
  if (hasRead) return "Read";
  
  return "None";
};

console.log("create =>",               caluclateAcess(["create"])                  );
console.log("read =>",                 caluclateAcess(["read"])                    );
console.log("update =>",               caluclateAcess(["update"])                  );
console.log("create, read =>",         caluclateAcess(["create", "read"])          );
console.log("update, read=>",          caluclateAcess(["update", "read"])          );
console.log("create, update =>",       caluclateAcess(["create", "update"])        );
console.log("create, read, update =>", caluclateAcess(["create", "read", "update"]));
console.log("<nothing> =>",            caluclateAcess([])                          );

The READ, CREATE, and UPDATE variables are not needed, since they are only used once, so I inlined them to shorten the code even more.

However, if there really is no possibility for an empty array of access properties, then that implies that the read access is completely inconsequential. It's impossible to not have it. So, checking for it can be entirely skipped and the default return value of the function can be changed from "None" to "Read". However, my personal preference is to keep the check - it doesn't hurt any of the functionality and the implementation produces no deviation from the specs. If the specs change in the future, or there is some sort of bug, it's probably better to not automatically grant read access.

Sign up to request clarification or add additional context in comments.

2 Comments

thank you for the good description, but little bit confused on how Update and Create combination is working
@DILEEPTHOMAS if you have both create and update access privileges, then you get "ALL". The read privilege is inconsequential, since if you have either create or update, you automatically have read, as well, therefore but ["create", "update"] and ["create", "update", "read"] are the same as just checking for create and update.

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.