-1
\$\begingroup\$

I am putting together a couple code samples for prospective employers and would love some feedback. I am still pretty junior so I am not sure what constitutes a good code sample and the do's and dont's.

The code sample shows both the original version of the code which is currently in production (at my previous company), and a refactored version, which is not in production and untested.

// This code is part of a piece of functionality I wrote in which double-booking for reservations is handled
// on the front-end. There is a series of modals that fire to guide a user through the process and gives
// options to override a previous reservation, keep it, warns of any fee's, and proceeds to rebook if a user overrides.

// I used nested promises to manage the asynchronicity issues and prevent the modals from firing at the same time, 
// and to make sure they were firing in the proper order. Just below this you will find a recently refactored 
// version of this code (not in production) in which I separated the promises, and flattened the chain - making it 
// more readable, allowing for better error handling (if needed), and improving manageability. 

// ----------------- original (in production) ----------------------------------------- //

BookingHandler.handlePromise(Booking.book())
.catch(function BookingHandlerReject(data) {
    if (data.response.status === 412) {
        ChangeCancel.dblBookingFlow = true;

        //first promise
        ChangeCancel.userReservations().then(function(userReservations) {
            var reservations = userReservations.reservations; 
            var reservationId = data.response.data.specs.reservation_id;
            var venueName = data.response.data.venue.name;

            //second promise
            ChangeCancel.alertExistingReservation(venueName, reservationId, reservations).then(function() {
                ChangeCancel.rebook = true; 

                //third promise
                ChangeCancel.cancel().then(function() {
                    if (ChangeCancel.skipSecondModal) { 
                        book();
                        ChangeCancel.dblBookingFlow = false;
                        ChangeCancel.skipSecondModal = false;
                    } else if (!ChangeCancel.inChangeFlow) { 
                        if (ChangeCancel.continueWithRebooking) {
                            book();
                            ChangeCancel.isReplacingReservation = false;
                            ChangeCancel.dblBookingFlow = false;
                            ChangeCancel.skipSecondModal = false;
                        }
                    }
                });
            }); 
        });         
    } else {
        thisCtrl.messages.push('There was a problem while trying to book this reservation.');
        thisCtrl.messages.push(data.response.data.message);
    }
});




// ----------------- refactored (not in production) ----------------------------------------- //

BookingHandler.handlePromise(Booking.book())
.catch(function BookingHandlerReject(data) {    
    if(data.response.status === 412) {
        var alertConflictingReservation, 
            getUserReservations,
            overrideConflictingReservation; 

        ChangeCancel.dblBookingFlow = true;

        getUserReservations = function() {
            return ChangeCancel.userReservations().then(function(reservationsData){
                this.reservations = reservationsData;
                this.conflictingReservationId =  data.response.data.specs.reservation_id;
                this.conflictingVenueName = data.response.data.venue.name;

                return reservations;
            }, function(error){ $log.error(error); });
        }; 
        alertConflictingReservation = function() {
            ChangeCancel.rebook = true;
            return ChangeCancel.alertExistingReservation(this.conflictingVenueName, this.conflictingReservationId, this.reservations).then(function(alertData){
                return alertData;
            }, function(error){ $log.error(error); });
        };
        overrideConflictingReservation = function() {
            return ChangeCancel.cancel().then(function(overrideData) {  
                if (ChangeCancel.skipSecondModal) { 
                    book();
                    ChangeCancel.dblBookingFlow = false;
                    ChangeCancel.skipSecondModal = false;                   
                } else if (!ChangeCancel.inChangeFlow) { 
                    if (ChangeCancel.continueWithRebooking) {
                        book();
                        ChangeCancel.isReplacingReservation = false;
                    }
                }
                return overrideData;
            }, function(error){ $log.error(error); });
        }; 

        //call promise chain
        getUserReservations()
        .then(alertConflictingReservation)
        .then(overrideConflictingReservation);

    } else {
        thisCtrl.messages.push('There was a problem while trying to book this reservation.');
        thisCtrl.messages.push(data.response.data.message);     
    }
});
\$\endgroup\$
1
\$\begingroup\$

you have an oddly nested if statement that you can un-nest a little bit

    overrideConflictingReservation = function() {
        return ChangeCancel.cancel().then(function(overrideData) {  
            if (ChangeCancel.skipSecondModal) { 
                book();
                ChangeCancel.dblBookingFlow = false;
                ChangeCancel.skipSecondModal = false;                   
            } else if (!ChangeCancel.inChangeFlow) { 
                if (ChangeCancel.continueWithRebooking) {
                    book();
                    ChangeCancel.isReplacingReservation = false;
                }
            }
            return overrideData;
        }, function(error){ $log.error(error); });
    };

your else if condition could be added to so that you don't have to have another level of indentation in your code.

overrideConflictingReservation = function() {
    return ChangeCancel.cancel().then(function(overrideData) {  
        if (ChangeCancel.skipSecondModal) { 
            book();
            ChangeCancel.dblBookingFlow = false;
            ChangeCancel.skipSecondModal = false;                   
        } else if (!ChangeCancel.inChangeFlow && ChangeCancel.continueWithRebooking) {
            book();
            ChangeCancel.isReplacingReservation = false;
        }
        return overrideData;
    }, function(error){ $log.error(error); });
}; 

in the original it is the same way

//third promise
ChangeCancel.cancel().then(function() {
    if (ChangeCancel.skipSecondModal) { 
        book();
        ChangeCancel.dblBookingFlow = false;
        ChangeCancel.skipSecondModal = false;
    } else if (!ChangeCancel.inChangeFlow && ChangeCancel.continueWithRebooking) { 
        book();
        ChangeCancel.isReplacingReservation = false;
        ChangeCancel.dblBookingFlow = false;
        ChangeCancel.skipSecondModal = false; 
    }
});
| improve this answer | |
\$\endgroup\$

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.