HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Booking validation in a big if-elseif block

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
bookingblockvalidationbigelseif

Problem

I have pasted a part of my event handler code block. I understand this code really looks like a mess specially the object that is created every time the message is changing. Is cleanup possible in this code? Any inputs will help.

```
btn.addEventListener('click', procs.debounce(bookDiningBtnHandler, 400));
var bookDiningBtnHandler = function() {
btnContext = this, message = {};
btnContext.touchEnabled = false;
btnContext.children[0].bubbleParent = 'false';
hideKeyBoard();
// Avoid overlap of keyboard with the prompt..
removeDatePicker();
// Avoid overlap of picker with the prompt..

var isValidEmail = procs.checkValidEmail(email.value);
var isIntPositive = procs.isPositiveInteger(guest.value);

if (name.value != '' && isValidEmail != false && isIntPositive != false && guest.value > 0 && date.text != 'Date' && time.text != 'Time') {
if (guest.value > guestLimit) {
message = {
"ref" : 0,
"title" : "Attention",
"text" : "Booking for more than 6, Please book through XYZ!"
};
customAlert(message);
return;
}
booking = {
"action" : "diningBook",
"emailReservations" : Alloy.Globals.data.dining.ER,
"name" : name.value,
"email" : email.value,
"guest" : guest.value,
"date" : date.text,
"time" : time.text
};

if(checkLastBooked(booking, Alloy.Globals.BDD)){
message = {
"ref" : 0,
"title" : "Attention",
"text" : "Bookings were made earlier!"
};
customAlert(message);
return;
}
Alloy.Globals.BDD = booking;
procs.sendEmail(booking, function(e) {
treatTheAnswer(e);
customAlert(message);
//Ti.API.info('email passed!');

Solution

Conditionals

isValidEmail != false && isIntPositive != false

I'd make that isValidEmail == true && isIntPositive == true. You could even short these to isValidEmail && isIntPositive. This makes your code more readable AND uses less characters (less that has to be read). It's a win-win.

var isIntPositive = procs.isPositiveInteger(guest.value);
... isIntPositive != false && guest.value > 0 ...


I don't know what isPositiveInteger does, but I have the feeling this check (guest.value > 0) is redundant. If you don't allow 0 then explicitly decline that value with guest.value != 0.

Duplication

You have a lot of message = { ... }. I'd create a separate function that takes the arguments. Then I'd create separate functions for the constants to be wrapped in, so you get things like this:

function createInvalidEmailMessage(){
    return createMessageObject(1, "Error", "Invalid Email Address. Please re-enter valid Address!");
}
function createMessageObject(ref, title, text){
    return {
        "ref": ref,
        "title": title,
        "text": text
    };
}

Code Snippets

var isIntPositive = procs.isPositiveInteger(guest.value);
... isIntPositive != false && guest.value > 0 ...
function createInvalidEmailMessage(){
    return createMessageObject(1, "Error", "Invalid Email Address. Please re-enter valid Address!");
}
function createMessageObject(ref, title, text){
    return {
        "ref": ref,
        "title": title,
        "text": text
    };
}

Context

StackExchange Code Review Q#64973, answer score: 2

Revisions (0)

No revisions yet.