1
\$\begingroup\$

I have a method that converts characters to numbers. It expects a single alphabetical character and returns the equivalent number. For example, if A is provided it returns 1, for B it returns 2. If an array is passed, it returns the message "array not accepted".

function characterToNumberCoverter(character) {
  if (Array.isArray(character)) {
    return { status: "failure", message: "array not accepted" };}
  character = character.toUpperCase();
  let myRegExp = /[A-Z]/;
  if (myRegExp.test(character)) {
    return character.charCodeAt(0) - 64;}}

The method returns an object if the input is invalid and an integer if the input is valid. The method is invoked like this:

let intConvertedInt=characterToNumberCoverter('A');

This works fine when the input is valid. What is a better way to handle the use case where an invalid input is passed? Example in which the result will be an object:

    let intConvertedInt=characterToNumberCoverter({});

Are there any other function design patterns that I should be following?

\$\endgroup\$
  • 1
    \$\begingroup\$ Please use JS formatting, not that of java. typical standards are that { are not on a new line. \$\endgroup\$ – FreezePhoenix Oct 22 '18 at 14:03
  • \$\begingroup\$ Why would you ever call this function with an array? Please provide the context for this code. \$\endgroup\$ – 200_success Oct 22 '18 at 14:10
  • \$\begingroup\$ @200_success I am trying to release methods like this as utility library. I provided documentation, but at times the developer sends in inputs like undefined,'',{},[] so I want to make my code return meaningful messages that the developer can make use of to fix it. \$\endgroup\$ – Ananda Oct 22 '18 at 14:14
  • \$\begingroup\$ @Ananda If the developer is putting in things that don't make sense, one should question the sanity of the developer. It shouldn't be up to you to make special return cases for anything a dev might throw at you. \$\endgroup\$ – FreezePhoenix Oct 23 '18 at 18:13
2
\$\begingroup\$

Why not just compare directly to the charcode?

function characterToNumberCoverter(character) {
  let code = character.toUpperCase().charCodeAt(0);
  if (code > 64 && code < 91) return code - 64
  return 0
}
\$\endgroup\$
  • \$\begingroup\$ my question was not about the code. It was a over simplified version of my code. I have already handled them in my actual code which is not relevant to the question I have raised. \$\endgroup\$ – Ananda Oct 23 '18 at 13:24
  • \$\begingroup\$ Since you return 1 for A, for all invalid inputs you can just return 0, which is an invalid alphabet index. \$\endgroup\$ – Calak Oct 23 '18 at 13:46
2
\$\begingroup\$

There are a couple of ways I would approach this.

Personally I think that someone passing an array is a coding error and I would just raise an exception in that case instead of complicating the interface.

If you do want to keep it as a return result then I would try and keep the return type consistent. i.e. I would return something like {success: true, character:'1'} in the case of success and something like { success: false, error: "array not accepted" } in the case of failure. This makes it easy for the caller to test the result (testing the result type is a bad practice IMO).

If you are concerned about bad input then I would test for any type of bad input. This could be as simple as if (character && character.charCodeAt) { ...

Lastly you should provide a return value for all possible paths. In your case you don't return anything if the character is not between 'a' and 'z'.

\$\endgroup\$
  • \$\begingroup\$ "testing the result type is a bad practice IMO" me too. But is there any approach to handle this. For example log the error and return -1 for invalid inputs. Lodash handles similar situation with the text "infinity" \$\endgroup\$ – Ananda Oct 23 '18 at 13:28
  • \$\begingroup\$ That is certainly an option you can also return undefined or an empty string (both of which are falsy and easy to test). Otherwise you can use the solution in the thrid paragraph and simply do something like if (result.success) { console.log(result.character) }. However my view on this is that an array is an illegal input / calling error and I would throw an exception. i.e. the caller needs to provide valid data. \$\endgroup\$ – Marc Rohloff Oct 23 '18 at 17:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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