CODEWITHSUNDEEP
javascript
amamam
amamam

Reputation: 19

Javascript - multiple complex if statements

Im struggling with multiple complex statements in javaScript and wondered if anyone could point me in the right direction.

    function findFlights()
    {
    var yourDestination = readTheDestination();
    var yourAirline = readTheAirline();
    var yourFare = readTheFare();


    if (yourDestination == 'Choose your destination')
        {
        displayMessage('<B>Please choose a destination city from the Destination menu and then click Find flights again.</B>');
        }
    else
        {
        var destinationTime, destinationOperator, destinationFare;
        var message = '<B>You asked about flights to ' + yourDestination + '</B><BR>' + "";

        for (var i=0; i < flightTimes.length; i++)                      //flight destinations
        {
        if    // statement                                              // IF flight:
           ((flightDestinations[i] == yourDestination &&                // destination = selected destination &
            yourAirline == 'Any airline' &&                             // airline = any airline &
            yourFare == 'Any price'))                                    // fare <= chosen fare
            ||                                                          // OR
           (flightDestinations[i] == yourDestination &&                // destination = selected destination &
           yourAirline == flightOperators[i] &&                        // airline = chosen airline &
           yourFare <= flightFares[i]))                                // fare <= chosen fare

            {
            destinationTime = flightTimes[i];
            destinationOperator = flightOperators[i];
            destinationFare = flightFares[i];



            message += destinationTime + ' ' + destinationOperator + '. £' + destinationFare + '<BR>';
            displayMessage(message);

            }
        }
        else if (flightDestinations[i] == yourDestination && 
                flightOperators[i] != yourAirline && 
                flightFares[i] != yourFare)
            {
            displayMessage('There are no flights to ' + yourDestination + ' with ' + yourAirline + '. Please select Any Airline and try again.');
            }
        }

This is what I have so far and its making me gray.

Upvotes: 1

Views: 15447

Answers (4)

Castrohenge
Castrohenge

Reputation: 8993

Refactor complex code to function

If you have complex if statements try and wrap them up in functions. So 

(flightDestinations[i] == yourDestination &&
yourAirline == 'Any airline' &&
yourFare == 'Any price')

could become 

function YourDestinationIsTheSameForAnyAirlineOrPrice(flightDestination, yourDestination, yourAirline, yourFare){
    return flightDestination == yourDestination &&
        yourAirline == 'Any airline' &&
        yourFare == 'Any price';
}

// And called from if
if (YourDestinationIsTheSameForAnyAirlineOrPrice(flightDestinations[i], yourDestination, yourAirline, yourFare)) {}

Rather than trying to decipher the if statement you have a function name telling you what it does.

Use an object over multiple arrays

Specific to your example I would also try and create a single flight object that contains the destination, time and airline. eg:

var flight = {
    destination = "London",
    operator = "BA",
    time = "18:00 UTC",
    fare = "£239829"
}

This should make the code more readable than using multiple arrays. Example:

destinationTime = flightTimes[i];
destinationOperator = flightOperators[i];
destinationFare = flightFares[i];

message += destinationTime + ' ' + destinationOperator + '. £' + destinationFare + '<BR>';

// Using an object
message += flight.time + ' ' + flight.operator + '. £' + flight.fare + '<br />';

Return early

Finally I would break out of the function as soon as possible. So use:

if (yourDestination == 'Choose your destination') {
    displayMessage('<B>Please choose a destination city from the Destination menu and then click Find flights again.</B>');
    return;
}

instead of an if...else. I personally find this more readable, so feel free to ignore this.

Upvotes: 7

Zafer
Zafer

Reputation: 2190

Although I think that this is not a necessary tuning, you can write the code as follows:

if (yourDestination == 'Choose your destination') {
    displayMessage('<B>Please choose a destination city from the Destination menu and then click Find flights again.</B>');
    return;
}

This helps you remove the else and its brackets {...}.

You can change the for loop to reduce the code size:

if(flightDestinations[i] != yourDestination) continue;

So that, you don't need to write this condition repeatedly.

Upvotes: 0

palswim
palswim

Reputation: 12140

You've mismatched your parenthesis here:

((flightDestinations[i] == yourDestination &&                // destination = selected destination &
 yourAirline == 'Any airline' &&                             // airline = any airline &
 yourFare == 'Any price'))                                    // fare <= chosen fare
 ||                                                          // OR
(flightDestinations[i] == yourDestination &&                // destination = selected destination &
yourAirline == flightOperators[i] &&                        // airline = chosen airline &
yourFare <= flightFares[i]))                                // fare <= chosen fare

Change yourFare == 'Any price')) to yourFare == 'Any price').

Upvotes: 1

palswim
palswim

Reputation: 12140

I'm not sure what the question is, but you should use more consistent indentation and the complex if statements will definitely become clearer.

My rules:

  • use tabs to indent your lines of code according to scope (e.g. +1 tab while within curly braces, if statements, etc.)
  • do not use tabs for anything else; use spaces for alignment, multiple spaces if necessary (so, use spaces to line up your conditions in the if statement if the statement spans multiple lines)

Upvotes: 0

Related Questions