Styjun

so this isn't really a problem rather a question if it's possible to shorten some code

My current code works although I feel it's too long and could maybe be done in a better way. Basically, I want to know if there's a way to select multiple elements to output the same alert instead of making a new if for every element, here's the code:

function newAdd() {
 if (document.getElementById("box1").value == "") 
 alert('Please define an Object!')
 return false;
 
 if (document.getElementById("box2").value == "") 
 alert('Please define an Object!')
 return false;
 
 if (document.getElementById("box3").value == "") 
 alert('Please define an Object!')
 return false;
 
 if (document.getElementById("box4").value == "") 
 alert('Please define a Time!')
 return false;
 
 if (document.getElementById("box5").value == "") 
 alert('Please define a Time!')
 return false;
 
 if (document.getElementById("box6").value == "") 
 alert('Please define a Time!')
 return false;
 
 if (document.getElementById("box7").value == "") 
 alert('Please define a Name!')
 return false;
 
 if (document.getElementById("box4").value.length == 1) 
 alert('Please define a valid Time!')
 return false;
 
 if (document.getElementById("box5").value.length == 1) 
 alert('Please define a valid Time!')
 return false;
 
 if (document.getElementById("box6").value.length == 1) 
 alert('Please define a valid Time!')
 return false;
 
 if (document.getElementById("box6").value.length == 2) 
 alert('Please define a valid Time! If last digit is unknown type 0.')
 return false;
 

I've tried ("box1, box2, box3") and that doesn't work, and getElementsByClassName wouldn't work because I wouldn't want to select the whole class in which all the text boxes are in.

If you noticed above I'm trying to select 3 text boxes at a time, only the last one is an exception.

The alerts are the same for the first three, the next three, and the last one though is only one.

If there isn't a more compact way to do this I guess I'll just live with it, but it just seems there's always a better way of doing it.

create an array of id's names and loop through it with one function.

var ids = ["one", "two", "three"];

ids.forEach((elem) => 
 if (document.getElementById(elem).getAttribute("value") == "") 
 alert('value empty');
 
)

<div id="one" value="">
 one
</div>
<div id="two" value="2">
 two
</div>
<div id="three" value="3">
 three
</div>

I'm in danger of commandeering your question a bit here, but here goes.

It's possible to output the same alert for multiple different fields, but I don't think that's actually what you want. If you output the same alert for each one, the user won't know which of these fields has a mistake in it. They won't like that.

It's also possible to shorten your code quite a bit, but I don't think fewer lines is what you really want either. I think what you really want is code that makes your intention clear and avoids needless repetition.

The solution I've written is actually slightly longer than yours, and it's harder to understand at first blush, but I think it does a better job at revealing the underlying structure of the work this task is doing. Once you understand the pattern it's easier to see how to change or extend it without introducing bugs. Anything that could be done differently for each input is written once for each input, and anything that needs to be handled the same for all of them is only written once.

(Your submitted code was written in ES2015, but I've written my response in the ES2016 dialect of Javascript because I'm more used to writing it and it allows for a cleaner syntax. If for whatever reason you can't or don't want to use ES2016, I've also posted an ES2015 version below.)

The structure is as follows:

1. We define some "testers" which are functions that run on a string and try to detect a validation problem (empty, invalid, etc.). If they detect the problem they were built to detect, they return true, otherwise false.


2. We define a function to turn a tester into a validator. A validator takes the same input, but if a problem is detected, it returns a complaint string, otherwise undefined. To call the function that turns a tester into a validator, the caller needs to provide a tester and a complaint string to use if the validation fails.


3. We define an object that maps element IDs to an array of validators that apply to those elements. The pattern from step 2 lets us reuse the empty-string validator on every input, even though different inputs will have different complaints if they are empty.


4. We loop over the IDs in the object. For each ID, we get the value for the corresponding input. We get the list of validators for the ID and run each of them on the value. If any of them come back with a complaint string, we alter the user with that complaint string and return. Otherwise we keep going until the loop ends.

ES2016:

function newAdd() 
 const nonEmptyTester = value => value === "";
 const validTimeTester = value => value.length === 1;
 const validDifferentTimeTester = value => value.length === 2;

 const makeValidator = (tester, failureMessage) => value => tester(value) ? failureMessage : undefined;

 const toValidate = 
 box1: [makeValidator(nonEmptyTester, "Please define an Object!")],
 box2: [makeValidator(nonEmptyTester, "Please define an Object!")],
 box3: [makeValidator(nonEmptyTester, "Please define an Object!")],
 box4: [
 makeValidator(nonEmptyTester, "Please define a Time!"),
 makeValidator(validTimeTester, "Please define a valid Time!")
 ],
 box5: [
 makeValidator(nonEmptyTester, "Please define a Time!"),
 makeValidator(validTimeTester, "Please define a valid Time!")
 ],
 box6: [
 makeValidator(nonEmptyTester, "Please define a Time!"),
 makeValidator(validTimeTester, "Please define a valid Time!"),
 makeValidator(validDifferentTimeTester, "Please define a valid Time! If last digit is unknown type 0.")
 ],
 box7: [validator(nonEmptyTester, "Please define a Name!")]
 ;

 for (let id in toValidate) 
 const element = document.getElementById(id);
 const value = element.value;
 const problems = toValidate[id].map(validator => validator(value));
 const firstProblemIfAny = problems.find(problem => problem !== undefined);

 if (firstProblemIfAny !== undefined) 
 alert(firstProblemIfAny);
 return false;
 
 

 return true;

ES2015:

function newAdd() 
 function nonEmptyTester(value) 
 return value === "";
 ;

 function validTimeTester(value) 
 return value.length === 1;
 ;

 function validDifferentTimeTester(value) 
 return value.length === 2;
 ;

 function makeValidator(tester, failureMessage) 
 return function (value) 
 return tester(value) ? failureMessage : undefined;
 ;
 ;

 var toValidate = 
 box1: [makeValidator(nonEmptyTester, "Please define an Object!")],
 box2: [makeValidator(nonEmptyTester, "Please define an Object!")],
 box3: [makeValidator(nonEmptyTester, "Please define an Object!")],
 box4: [makeValidator(nonEmptyTester, "Please define a Time!"), makeValidator(validTimeTester, "Please define a valid Time!")],
 box5: [makeValidator(nonEmptyTester, "Please define a Time!"), makeValidator(validTimeTester, "Please define a valid Time!")],
 box6: [makeValidator(nonEmptyTester, "Please define a Time!"), makeValidator(validTimeTester, "Please define a valid Time!"), makeValidator(validDifferentTimeTester, "Please define a valid Time! If last digit is unknown type 0.")],
 box7: [validator(nonEmptyTester, "Please define a Name!")]
 ;

 for (var id in toValidate) 
 var element = document.getElementById(id);
 var value = element.value;
 var problems = toValidate[id].map(function (validator) 
 return validator(value);
 );
 var firstProblemIfAny = problems.find(function (problem) 
 return problem !== undefined;
 );

 if (firstProblemIfAny !== undefined) 
 alert(firstProblemIfAny);
 return false;
 
 

 return true;

I hope you'll agree that even though this is more code, it's more readable. The object shows exactly what form validation you're doing with much less cruft, and it's immediately obvious where to go to change certain behaviour. For example:

• The testers handle the exact logic for what's acceptable for a certain type of input.


• The validator object handles exactly which testers apply to which element, and how the elements complain if their requirements are not met.


• The loop at the end defines what to do if a complaint is found. For example, you might want to print the complaint to an element on the screen instead of calling alert. Now this would require a change in only one place!

Finally, if you're sure you want to reuse the complaint strings across elements, you could do something like this:

var objectValidator = makeValidator(nonEmptyTester, "Please fill in all fields!");

var toValidate = 
 id1: [objectValidator],
 id2: [objectValidator],
 ...
;