Javascript function to validate contents of an array

Question

Here's what is asked:

validItems(items) – this function receives a string array of items which are to be for a customer. The function returns an empty string indicating all item codes in the array are valid; otherwise the function returns the first invalid item code in the array. All item codes must be selected from the item codes provided. They are: IT00, O144, 6A1L, 4243, O3D5, 44SG, CE64, 54FS and 4422.

This is what I've done so far:

 function validItems(items) {
     
      var error = false;
    
      for (i = 0; i < items.length; i++) {
    
        if (error == false) {
    
          if (items[i] != "IT00" ||
            items[i] != "0144" ||
            items[i] != "6A1L" ||
            items[i] != "4243" ||
            items[i] != "O3D5" ||
            items[i] != "44SG" ||
            items[i] != "CE64" ||
            items[i] != "54FS" ||
            items[i] != "4422") {
    
            error = items[i];
    
          } else {
    
            error = false;
          }
        } else {
          if (error != false) {return error;} else {return "";}
        }
    
      }
    
    }
    
    var items = ["IT00","0144","6A1L"];
    alert(validItems(items));

It keeps on returning IT00. What am I doing wrong?

Mulan · Accepted Answer · 2019-04-27 20:25:14Z

1

What you'll notice here is that there is zero complexity. Each function below takes a couple arguments and does one simple task. It's very easy to see what each function does at first glance.

// your data
const validItems = [
  "0144", "6A1L", "4243", "O3D5", "44SG", "CE64", "54FS", "4422"
];

// some reusable functions
const all = f => xs => xs.every(f);
const comp = f => g => x => f(g(x));
const neq = y => x => x !== y;
const indexOf = xs => x => xs.indexOf(x);
const elem = xs => comp(neq(-1))(indexOf(xs))

// your helpers
const validateItems = all(elem(validItems));

// test it out
console.log( validateItems(["0144", "6A1L"]) ); // true
console.log( validateItems(["0144", "CAKE"]) ); // false

edited Apr 27, 2019 at 20:25

answered Jun 22, 2015 at 6:55

Mulan

136k35 gold badges240 silver badges276 bronze badges

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

Comments

Arun P Johny · Accepted Answer · 2015-06-22 06:43:31Z

0

You can use a simple array based test like

var validCodes = ['IT00', 'O144', '6A1L', '4243', 'O3D5', '44SG', 'CE64', '54FS', '4422'];

function validItems(items) {
  for (var i = 0; i < items.length; i++) {
    if (validCodes.indexOf(items[i]) == -1) {
      return items[i];
    }
  }
  return '';
}

var items = ["IT00", "O144", "6A1L"];
alert(validItems(items));

To make your code work

function validItems(items) {

  var error = false;

  for (i = 0; i < items.length; i++) {
    console.log(items[i], error)

    if (error == false) {

      //need to use && since otherwise one value cann't satisfy all these conidtions
      if (items[i] != "IT00" && items[i] != "0144" && items[i] != "6A1L" && items[i] != "4243" && items[i] != "O3D5" && items[i] != "44SG" && items[i] != "CE64" && items[i] != "54FS" && items[i] != "4422") {
        //if current item is not matching assign it to the error and break the loop
        error = items[i];
        break;
        //you can really return from here, not need to use the error variable also
      }
    }
  }
  //this should be outside of the loop
  //if there is an errro return the error string
  if (error != false) {
    return error;
  } else {
    return "";
  }
  return '';
}
var items = ["IT00", "0144", "6A1L"];
alert(validItems(items));

edited Jun 22, 2015 at 6:43

answered Jun 22, 2015 at 6:25

Arun P Johny

389k68 gold badges532 silver badges532 bronze badges

2 Comments

Tschallacka Over a year ago

... you have 209k points and you did not put the if in an Array

Arun P Johny Over a year ago

@MichaelDibbets see the first solution.... the second was just a correction to original code

Stefan · Accepted Answer · 2015-06-22 06:24:06Z

0

According to your code it is correct that it is outputting IT00. You are using the OR selector which means if the string is IT00, then it is not 0144 or 6A1L. Your intention is to exclude ALL so you are looking for values which are not IT00 AND not 0144 ANd not 6A1L, etc. etc.

use the AND:

if (items[i] != "IT00" &&
    items[i] != "0144" &&
    items[i] != "6A1L" &&
    items[i] != "4243" &&
    items[i] != "O3D5" &&
    items[i] != "44SG" &&
    items[i] != "CE64" &&
    items[i] != "54FS" &&
    items[i] != "4422") {

When you understand the basic of this logic, also try to rewrite your code. An array of allowed values is for example a bit more tidy ;-)

answered Jun 22, 2015 at 6:24

Stefan

1,2586 silver badges23 bronze badges

1 Comment

Tschallacka Over a year ago

Using an if statement is wrong. With a long list of items you do not want to edit a long tedious if statement... use an array.

Daniël Camps · Accepted Answer · 2015-06-22 06:25:25Z

0

Your first item returns true on your if statement. If your first item is "ITOO", The first match of you make is:

items[i] != "0144"

your code then says

error = items[i]; //which is "ITOO"

and then you return

error

which is the first item "ITOO"

answered Jun 22, 2015 at 6:25

Daniël Camps

1,8091 gold badge25 silver badges38 bronze badges

Comments

amisax · Accepted Answer · 2015-06-22 06:30:39Z

0

Your Or condition should have "==" instead of "!=".

Which means -> If "the give code" is same as "any of the recognized codes" then recognize it otherwise drop it.

Currently your condition means -> If "the given code" is not same as "any of the recognized code" then recognize it. This condition will always be true

answered Jun 22, 2015 at 6:30

amisax

1262 bronze badges

Comments

Tschallacka · Accepted Answer · 2015-06-22 06:33:59Z

You had some basic coding errors in your code. I've modified your code and put in the comments where I saw room for improvement.

Basically your if else statements were redundant. If you simply exit the function by returning the faulty thing you already get the desired result.

No need to keep looping if we have found a mismatch. In a function where you'd need to do additional checks after finding a fault you would use break and then do your logic on error if error !== false

 function validItems(items) {
              // create a valid items array. This will make maintaining valid item codes easier. and keep your code readable.
              var valid = ["IT00","0144","6A1L","4243","O3D5","44SG","CE64","54FS","4422"];
              var error = false;
            
              for (i = 0; i < items.length; i++) {
                // Type safe test. Always use 3 === isntead of == your test would have returned true on eveyrthing.
                if (error === false) {
            
                  if(valid.indexOf(items[i]) === -1) {
                    // immedeately escape
                    return items[i];
            
                  } /*else {// Totally uneccesary
                    error = false;
                  }
                } else {
                  // not needed here. this also escaped your loop after first iteration.
                  if (error !== false) {return error;} else {return "";}
                }*/
              
              }
              // we return here because we know the loop is done then.
              return error;
            }
            
            var items = ["IT00","0144","6A1L"];
            alert(validItems(items));

Collectives™ on Stack Overflow

Javascript function to validate contents of an array

6 Answers 6

Comments

2 Comments

1 Comment

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

6 Answers 6

Comments

2 Comments

1 Comment

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related