2
\$\begingroup\$

I'd like some feedback on the readability and style of the following code. It's a simple Node.js app that uses the Mailchimp API to add emails to a list. The client side handles SVGs, sign-up validation, and a contact modal with validation.

index.js (Node entry point)

const express = require('express');
const bodyParser = require('body-parser');
const request = require('superagent');
const app = express();

app.use(bodyParser.json());
app.use(bodyParser.urlencoded({ extended: true }));
app.use(express.static(__dirname + '/public'));
app.set('port', (process.env.PORT || 5000));
app.set('views', __dirname + '/views');
app.set('view engine', 'ejs');

app.get('/', function (req, res) {
    res.render('index');
});

app.listen(app.get('port'), function () {
    console.log('Node app is running on port', app.get('port'));
});

var mailchimpInstance = process.env.MAILCHIMP_INSTANCE,
    listUniqueId = process.env.MAILCHIMP_LIST_ID,
    mailchimpApiKey = process.env.MAILCHIMP_API_KEY;

app.post('/signup', function (req, res) {
    request
        .post('https://' + mailchimpInstance + '.api.mailchimp.com/3.0/lists/' + listUniqueId + '/members/')
        .set('Content-Type', 'application/json;charset=utf-8')
        .set('Authorization', 'Basic ' + new Buffer('any:' + mailchimpApiKey).toString('base64'))
        .send({
            'email_address': req.body.email,
            'status': 'subscribed'
        })
        .end(function (err, response) {
            if (response.status < 300 || (response.status === 400 && response.body.title === 'Member Exists')) {
                res.send(true);
            } else {
                res.send(false);
            }
        });
});

main.js (client side)

$(document).ready(function () {
    emailjs.init("user_e00V3mHLZYEOvZAHpjKOt");

    $('img[src$=".svg"]').each(function () {
        var $img = jQuery(this);
        var imgURL = $img.attr('src');
        var attributes = $img.prop("attributes");

        $.get(imgURL, function (data) {
            // Get the SVG tag, ignore the rest
            var $svg = jQuery(data).find('svg');

            // Remove any invalid XML tags
            $svg = $svg.removeAttr('xmlns:a');

            // Loop through IMG attributes and apply on SVG
            $.each(attributes, function () {
                $svg.attr(this.name, this.value);
            });

            // Replace IMG with SVG
            $img.replaceWith($svg);
        }, 'xml');
    });
});

$('#signup-form').on('submit', function () {
    var data = {};
    var emailInput = document.getElementById('email');
    data.email = emailInput.value;

    $.ajax({
        type: 'POST',
        data: JSON.stringify(data),
        cache: false,
        contentType: 'application/json',
        datatype: 'json',
        url: '/signup',
        success: function (returns) {
            if (returns) {
                showSuccess('#email')
                swal('Congratulations!', 'Please check your email to confirm your subscription.', 'success')
            } else {
                showError('#email')
                swal({
                    title: 'Error!',
                    text: 'Please check your email address again.',
                    type: 'error'
                });
            }
        }
    });

    return false;
});

$('.toggle-modal').on('click', function () {
    $('#contact-modal').toggleClass('is-active');
});

$('#contact-button').on('click', function () {
    var email = document.getElementById('email-input').value;
    var name = document.getElementById('name-input').value;
    var message = document.getElementById('message-input').value;

    var valid = true
    valid = isValid(name, '#name-input');
    valid = isValid(email, '#email-input');
    valid = isValid(message, '#message-input');

    if (valid) {
        $('#contact-button').toggleClass('is-loading');
        emailjs.send(
            "gmail",
            "contact_me", {
                "reply_to": email,
                "from_name": name,
                "message_html": message
            }
        ).then(function (response) {
            resetForm();
            swal('Thanks!', 'You\'ll be hearing from us shortly.', 'success')
        }, function (err) {
            resetForm();
            swal({
                title: 'Error!',
                text: 'Please try again later.',
                type: 'error'
            });
        });
    }
});

function resetForm() {
    // Remove loading state and close the modal
    $('#contact-button').toggleClass('is-loading');
    $('#contact-modal').toggleClass('is-active');

    var ids = ['#name-input', '#email-input', '#message-input']
    for (var i = 0; i < ids.length; i++) {
        input = ids[i]
        $(input).val('');
        $(input).removeClass('is-success');
        $(input).removeClass('is-danger');
        $(input + '-success').addClass('is-hidden');
        $(input + '-warning').addClass('is-hidden');
    }
}

function isValid(value, input) {
    if (isEmpty(value) || isBlank(value)) {
        showError(input)
        return false
    } else {
        showSuccess(input)
        return true;
    }
}

function showError(input) {
    $(input).removeClass('is-success');
    $(input).addClass('is-danger');
    $(input + '-success').addClass('is-hidden');
    $(input + '-warning').removeClass('is-hidden');
    $(input + '-button').addClass('is-danger');
}

function showSuccess(input) {
    $(input).removeClass('is-danger');
    $(input).addClass('is-success');
    $(input + '-success').removeClass('is-hidden');
    $(input + '-warning').addClass('is-hidden');
    $(input + '-button').removeClass('is-danger');
    $(input + '-button').addClass('is-primary');
}

function isEmpty(str) {
    return (!str || 0 === str.length);
}

function isBlank(str) {
    return (!str || /^\s*$/.test(str));
}

Full code available here - https://github.com/leerob/Drink

\$\endgroup\$
2
\$\begingroup\$

I've never worked with JQuery, but I can give you a few tips on the node part:

First, define a project structure. Separate the code in different files and directories for easier reading as well as making it more scalable:

src
|_ endpoints
|   |_ routes.js
|   |_ mailchimp.endpoint.js
|
|_ config
|   |_ db.config.js
|
|_ app.js

Secondly, here are a few overall tips:

  • Prefer the use of let/const always over var. You won't be needing var again. Ever.
  • Use ES6 arrow functions and, in case they're a one-liner, avoid the brackets.
  • Use the express router
  • Use use strict as a best practice.

app.js

'use strict';

const express = require('express');
const bodyParser = require('body-parser');
const request = require('superagent');
const app = express();

app.use(bodyParser.json());
app.use(bodyParser.urlencoded({ extended: true }));
app.use(express.static(__dirname + '/public'));
app.set('port', (process.env.PORT || 5000));
app.set('views', __dirname + '/views');
app.set('view engine', 'ejs');

require('./endpoints/routes.js')(app);

app.get('/', (req, res) => res.render('index'));

app.listen(app.get('port'), () => console.log('Node app is running on port', app.get('port')));

module.exports = app;

routes.js

'use strict';

const app = require('express');

const mailchimp = require('./mailchimp.endpoint.js');

const routes = (app) => {
    app.use('', mailchimp);
};

module.exports = routes;

mailchimp.endpoint.js

'use strict';

const express = require('express'),
    router = express.Router();

const mailchimpInstance = process.env.MAILCHIMP_INSTANCE,
    listUniqueId = process.env.MAILCHIMP_LIST_ID,
    mailchimpApiKey = process.env.MAILCHIMP_API_KEY;

const requestMailchimp = (req, res) => {
    request
        .post('https://' + mailchimpInstance + '.api.mailchimp.com/3.0/lists/' + listUniqueId + '/members/')
        .set('Content-Type', 'application/json;charset=utf-8')
        .set('Authorization', 'Basic ' + new Buffer('any:' + mailchimpApiKey).toString('base64'))
        .send({
            'email_address': req.body.email,
            'status': 'subscribed'
        })
        .end((err, response) => {
            if (response.status < 300 || (response.status === 400 && response.body.title === 'Member Exists')) {
                res.send(true);
            } else {
                res.send(false);
            }
        });
};


router.post('/signup', requestMailchimp);
module.exports = router;
\$\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.