2
\$\begingroup\$

I'm looking for some general feedback about the layout and structure of my code. Basically just any feedback, as I'm not super happy about this code, but I'm not sure how to improve it further.

'use strict';

/* jshint node: true */

var fs         = require('fs');
var os         = require('os');
var nodemailer = require('nodemailer');
var rimraf     = require('rimraf');
var config     = require('../config');
var dropbox    = require('../lib/dropbox-sync');
var mysql      = require('../lib/mysql-backup');
var tarball    = require('../lib/tarball');
var transport  = nodemailer.createTransport('sendmail');

// This is the FILE name of the backup file
var backupFilename  = config.dropbox.mysql_prefix + 'backup-' + Date.now();

// This is the absolute file path to save the file to on Dropbox
var dropboxFilename = ['', config.server, config.dropbox.mysql_folder, backupFilename].join('/');

// Connect to Dropbox with our credentials
try {
  dropbox.connect({
    key: config.dropbox.key,
    secret: config.dropbox.secret
  });
} catch (err) {
  failWithDropboxError(err);
}

// Connect to MySQL with our credentials
try {
  mysql.connect({
    host: config.mysql.hostname,
    user: config.mysql.username,
    password: config.mysql.password
  });
} catch (err) {
  failWithMySQLError(err);
}

// Generate a MYSQL backup
mysql.backup(backupFilename, function (err, pathname, stats) {
  if (err) {
    return failWithMySQLError(err);
  }

  tarball.create(pathname, '/tmp/' + backupFilename + '.tar.gz', function (code) {
    rimraf.sync(pathname);

    dropbox.send('/tmp/' + backupFilename + '.tar.gz', dropboxFilename, function (err, response) {
      fs.unlinkSync('/tmp/' + backupFilename + '.tar.gz');

      if (err) {
        failWithDropboxError(err);
      } else {
        console.log(response);
        console.log('The MySQL backup has been completed successfully.\n');
        console.log('Backed up %d from %d databases.\n', stats.tables, stats.databases);
        console.log('The backup file has been saved to your Dropbox: %s', '');
      }
    });
  });
});

function failWithMySQLError (err) {
  console.error('The MySQL backup job has encountered a fatal error and could not continue.\n');
  console.error('We were unable to create the MySQL backup.\n');
  console.error(err);

  process.exit(1);
};

function failWithDropboxError (err) {
  console.error('The MySQL backup job has encountered a fatal error and could not continue.\n');
  console.error('We were unable to sync the MySQL backup to Dropbox.\n')
  console.error(err);

  process.exit(1);
};
\$\endgroup\$
1
  • \$\begingroup\$ What are you not happy about? This code looks good enough. \$\endgroup\$ Commented Oct 8, 2013 at 18:51

1 Answer 1

3
\$\begingroup\$

You code looks good and well formatted. There are only a few minor issues:

  • It seems that you have tried to check it with JSHint but there are some issues that JSHint reports about your code. In particular, semicolons and unused variables.
  • It is not clear when the character "\n" is required in messages, and when it is not. It looks like sometimes you just forgot it.
  • This one looks inconsistent. It looks like there is an error:

    console.log('The backup file has been saved to your Dropbox: %s', '');
    
  • This actually a bad pattern:

    if (err) {
        return failWithMySQLError(err);
    }
    

    it shortens a code a little but adds impression that the value of the function is important, while it is not.

  • else is redundant here:

    if (err) {
        failWithDropboxError(err);
    } else {
    
  • and as you see, the long lines complicate the code review.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.