Avatar of John S.
John S.
Flag for United States of America asked on

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

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

JavaScriptjQuery

Avatar of undefined
Last Comment
ste5an

8/22/2022 - Mon
David S.

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.
Julian Hansen

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.
ASKER CERTIFIED SOLUTION
Julian Hansen

Log in or sign up to see answer
Become an EE member today7-DAY FREE TRIAL
Members can start a 7-Day Free trial then enjoy unlimited access to the platform
Sign up - Free for 7 days
or
Learn why we charge membership fees
We get it - no one likes a content blocker. Take one extra minute and find out why we block content.
Not exactly the question you had in mind?
Sign up for an EE membership and get your own personalized solution. With an EE membership, you can ask unlimited troubleshooting, research, or opinion questions.
ask a question
ste5an

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.
I started with Experts Exchange in 2004 and it's been a mainstay of my professional computing life since. It helped me launch a career as a programmer / Oracle data analyst
William Peck