We help IT Professionals succeed at work.

Boss wants this small script "cleaned up" - Help needed Plz

John S.
John S. asked
on
137 Views
Last Modified: 2018-09-14
I wrote a simple checkout script for our page at work. When one of the more experienced developers looked at my code, he made a comment "we need to discuss you cleaning up your code"

Ok, so I am assuming he means to re-factor it, or take a more object approach.

Can anyone help me make this look a bit more elegant?  Any help would be appreciated. Maybe I can keep him off my back tomorrow!  :)

/** Document Ready */



jQuery(function ($) {

    jQuery.ajaxSetup({
        beforeSend: function () {
            $('#mmpc-loader').show();
        },
        complete: function () {
            $('#mmpc-loader').hide();
        }
    });

    // Form Submitted
    $( "#btnPurchase" ).click( function ( e )
    {

        e.preventDefault();

        Products = (sessionStorage.getItem("product") === "") ? sessionStorage.getItem("product") : "Solopreuner";

        // Hard Code Price
        let Price = 40;

        /* API Endpoints */
        let mmpcApi = "/Product/ProcessPayment";
        let authNetApi = "/Product/ProcessPayment";

        
        let Workstations = $( "#numWorkStations" ).val();   // Qty
        let total = 40 * Workstations;

        let valid = true;  // for future use when validating

        let FirstName = $( "#firstName" ).val();
        let LastName = $( "#lastName" ).val();
        let CompanyName = $( "#companyName" ).val();
        let Title = $( "#usrTitle" ).val();
        let Email = $( "#email" ).val();
        let Address = $( "#address" ).val();
        let City = $( "#city" ).val();
        let State = $( "#state" ).val();
        let ZipCode = $("#zip").val();
        let Phone = $("#phone").val();

        let CcNum = $( "#ccnum" ).val();
        let CcExpMonth = $( "#expmonth" ).val();
        let CcExpYear = $( "#expyear" ).val();
        let CcCvv = $( "#cvv" ).val();



        //product model per device
        const ProductViewModel = function (title, price, billingCycle, quantity, recurring) {
            this.Title = title;
            this.Price = price;
            this.BillingCycle = billingCycle;
            this.Quantity = quantity;
            this.Recurring = recurring;
        };

          // Object for Authorize.NET
        var ObjAuthDotNet = {
            FirstName: FirstName,
            LastName: LastName,
            Email: Email,
            Address: Address,
            State: State,
            Zip: ZipCode,
            CardNumber: CcNum,
            ExpirationDate: CcExpMonth + "/" + CcExpYear,
            CardCode: CcCvv,
            Phone: Phone,
            Country: "USA",
            Product: new ProductViewModel(Products, Price, 1, Workstations, true)
        };

        var ObjPaymentConfirmation = {
            FirstName: FirstName,
            LastName: LastName,
            Email: Email,
            Product: sessionStorage.getItem("product"),
            ClientId: "123"  // hard code for now
        };

        // Duplicate Object For MMPC, we are going to need
        // to add data to it for our own purposes.
        var objMMPC = jQuery.extend(true, {}, ObjAuthDotNet);

        // Process the payment
        $.ajax({
            type: 'POST',
            url: mmpcApi,
            data: ObjAuthDotNet,
            dataType: 'json',
            encode: true
        })
            // promise callback
            .done(function (data) {

                console.log(data);

                // Check for Errors
                if (data.messages.resultCode === "Error") {

                    var paymentErrors = [];

                    for (var i = 0; i < data.messages.message.length; i++) {

                        // Invalid Card Number
                        if (code === "37") {
                            paymentErrors.push("You have entered an invalid card number. Please try again.");
                         // Card Declined
                        } else if (code === "145" || code === "152") {
                            paymentErrors.push("Your credit card was declined. Please try another card.");
                        // For Wells Fargo Only, Billing/Zip
                        } else if (code === "112") {
                            paymentErrors.push("Your billing state / zip code is invalid. Please try again.");
                        // Invalid Card COde
                        } else if (code === "78") {
                            paymentErrors.push("The card code you entered was incorrent. Please try again.");
                        } else {
                            /* Unspecified Error */
                            paymentErrors.push("There was an unspecified error. Please try again.");
                        }
                    }

                    // Notify User
                    alertify.alert("There was an error processing your card.");
                    return false;

  
                } else if (data.messages.resultCode === "Ok") {

                    // Successful, Send Thank You Email
                    $.ajax({
                        type: "POST",
                        url: "/Client/ThankYou",
                        data: ObjPaymentConfirmation,
                        dataType: "json"
                    });

                    // Let User Know
                    alert('Payment Successful'); // for now we'll just alert the user
                    //window.location = '/Product/ProcessPayment';  // redirect a user to another page

                } else {

                    // Unspecified Response. Report as error
                    alertify.alert('Error', 'There was an error processing your payment. Please try again.'); 
                    
                }

            });

    });

    //--------------------------------------------------

    $('#numWorkStations').on('change', function () {
        var tNumWorkstation = this.value;
        //alert(tNumWorkstation);
        UpdateProductHeader(tNumWorkstation);
        $("#total").text(40 * tNumWorkstation);

    });


    function UpdateProductHeader(wCount) {
        var prodHeaderElem = $("#productHeading");
        var headerText;

        if (wCount === 1) {
            headerText = "Solopreuner";
        }

        if (wCount >= 2) {
            if (wCount <= 50) {
                headerText = "Small Business";
            }
        }

        if (wCount > 50) {
            // Display contact modal, re-direct to contact page
            headerText = "Enterprise";
        }

        prodHeaderElem.text(headerText);
        sessionStorage.setItem('product', headerText);
    }

});

Open in new window

Comment
Watch Question

David S.Consultant & Challenge Subduer
CERTIFIED EXPERT
Top Expert 2009

Commented:
Only three things stick out to me. The first is only if-conditions were used in UpdateProductHeader but 2 else-if conditions seem quite applicable. Also in that function, why the nested if-condition instead of using an "&&"?

The second issue I would address is using the comma operator when declaring variables instead of using the "var" or "let" keyword for each one.

Thirdly, were is "Products" declared? You defined it on line 22.
CERTIFIED EXPERT
Most Valuable Expert 2017
Distinguished Expert 2019

Commented:
1. I would move constants out of the click handler
2. I would create a factory for the ObjAuthDotNet object. I would also consider setting up constant references to the input controls (assuming you use this code more than once) and then just access the .value elements of the HTML nodes.
3. I would put the .done code into separate functions - the reason being so you can easily see the flow of your click handler
- Instantiation (setting up the call
- The call itself
- Handling the response
  - Handling message output
  - Handling AJAX call to send email

The key thing you want is that your code looks readable - and anyone reading it can easily understand the flow without having to be bogged down in detail or scrolling up and down to see where they are or what they are doing.
CERTIFIED EXPERT
Most Valuable Expert 2017
Distinguished Expert 2019
Commented:
This one is on us!
(Get your first solution completely free - no credit card required)
UNLOCK SOLUTION
ste5anSenior Developer
CERTIFIED EXPERT

Commented:
Some general comments, Julian did a great detailed analysis:

I don't see any validation of the input data. When dealing with payment processing, you must validate.

Your original code contains a lot of inline comments. In many cases inline comments indicates blocks of code which should be separate methods. Cause then the method name can carry that information.

Another approach to this is to outline your workflow on paper. This gives you also a overview of the necessary methods needed.
Unlock the solution to this question.
Join our community and discover your potential

Experts Exchange is the only place where you can interact directly with leading experts in the technology field. Become a member today and access the collective knowledge of thousands of technology experts.

*This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.

OR

Please enter a first name

Please enter a last name

8+ characters (letters, numbers, and a symbol)

By clicking, you agree to the Terms of Use and Privacy Policy.