HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Edit/Details form in JavaScript

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
javascripteditformdetails

Problem

I have a Edit/Details form which has 4 user related fields. On click of Save, I save the edited fields to local storage (if supported) and display the same values in the Details view.

Below is the code which does that;

```
var UserDataObj = {
name:"John",
email:"john@example.com",
phone:"9999999999",
desc:"some description"
}

/**
* Check HTML5 Local Storage support
*/
function supports_html5_storage()
{
try {
window.localStorage.setItem( 'checkLocalStorage', true );
return true;
} catch ( error ) {
return false;
};
};

/**
* Save form data to local storage
*/
function saveFormData()
{
if (supports_html5_storage())
{
$(".formFieldUserData").each(function(){
UserDataObj[$(this).attr("name")] = $(this).val();
});

localStorage.setItem('UserDataObj', JSON.stringify(UserDataObj));
}
}

/**
* Set field values based on local storage
*/
function setFormFieldValues()
{
if (supports_html5_storage())
{
var retrievedUserDataObj = JSON.parse(localStorage.getItem('UserDataObj'));
if(retrievedUserDataObj)
{
$(".formFieldUserData").each(function(){
var currentKey = $(this).attr("name");
var retrievedValue = retrievedUserDataObj[$(this).attr("name")]
$("#"+currentKey).val(retrievedValue);
$("#"+currentKey+"Txt").html(retrievedValue);
});
}
else
{
$(".formFieldUserData").each(function(){
var currentKey = $(this).attr("name");
var userDataObjValue = UserDataObj[$(this).attr("name")]
$("#"+currentKey).val(userDataObjValue);
$("#"+currentKey+"Txt").html(userDataObjValue);
});
}
}
else
{
$(".formFieldUserData").each(function(){
var currentKey = $(this).attr("name");

Solution

-
You can start with how you code. Have them indented properly and checked by a quality-check tool. Code-review doesn't only deal with optimization, but also maintainability of your code and that includes making it readable.

-
After running it in JSHint, I found some minor problems in your code. Most notable are missing semi-colons. Although semi-colons in line endings are optional in JS, it's still best practice to include them. You could also avoid code packer/compressor errors by actually putting them in. This avoids your lines from being connected to the next line.

-
Use strict comparison as much as possible in JS. JS treats "falsy" values as false, which includes null, undefined, 0, false, a blank string and NaN. For example, `"" == false is actually equal.

-
Place your code in a namespace, or some inner-scope to avoid polluting the global scope.

For code related stuff:

(function (ns, $) {
  "use strict"; //prevents "bad practice"

  //variables needed for localStorage test
  var storage, fail, uid;

  //For flexibility, it would be best if your code 
  //wasn't build for a specific form only
  //instead, allow it to create instances and attach
  //to different forms with different default data
  function FormSave(selector, defaultValues) {
    this.defaultValues = defaultValues || {};
    this.dataName = selector;
    this.form = $(selector);
    this.userFields = $('.formFieldUserData', this.form);
  }

  //our save function
  FormSave.prototype.save = function () {

    var userData = {};

    //using the "early return" pattern to avoid further
    //code exectution if condition fails, and avoid
    //deep indentation
    if (!storage) {
      return;
    }

    //since each() assigns the current DOM element in
    //the collection as this, we can get name and value
    //directly from the DOM element, saving you function calls
    this.userFields.each(function () {
      userData[this.name] = this.value;
    });

    //set the items
    localStorage.setItem(this.dataName, JSON.stringify(userData));
  };

  //our setting function
  FormSave.prototype.set = function () {

    //cache the default values into this scope
    var defaultValues = this.defaultObject,
      retrievedValues;

    //if local storage is present, get and parse
    if (storage) {
      retrievedValues = JSON.parse(localStorage.getItem(this.dataName));
    }

    this.userFields.each(function () {

      //cache the current key
      var currentKey = this.name,
        //then the value to be used is:
        //if storage is present, use either the retrieved values or the default
        //if not, then use the present value or the default
        value = ((storage) ? retrievedValues[currentKey]: this.value) || defaultValues[currentKey];

      //for safe text insertion, use text() since it escapes the value first
      $("#" + currentKey + "Txt").text(value);
      $("#" + currentKey).val(value);
    });

    //our easy binding function exposed to our FormSave namespace
    ns.bind = function (selector) {
      return new FormSave(selector);
    };
  };

  //a quick test for localstorage
  //from: http://mathiasbynens.be/notes/localstorage-pattern
  try {
    uid = new Date();
    (storage = window.localStorage).setItem(uid, uid);
    fail = storage.getItem(uid) !== uid;
    storage.removeItem(uid);
    fail && (storage = false);
  } catch (e) {}

}(this.FormSave = this.FormSave || {}, jQuery));

//usage:

//bind this functionality to a form id'ed "someForm" with
//the given default values
var someForm = FormSave.bind('#someForm', {
  name: 'John',
  email: 'john@example.com',
  phone: '9999999999',
  desc: 'some description'
});

//then with the given reference, call save and set
someForm.save();
someForm.set();


When packed, it's this small!

```
(function (ns, $) {
"use strict";
var storage, fail, uid;

function FormSave(selector, defaultValues) {
this.defaultValues = defaultValues || {};
this.dataName = selector;
this.form = $(selector);
this.userFields = $('.formFieldUserData', this.form)
}
FormSave.prototype.save = function () {
var userData = {};
if (!storage) {
return
}
this.userFields.each(function () {
userData[this.name] = this.value
});
localStorage.setItem(this.dataName, JSON.stringify(userData))
};
FormSave.prototype.set = function () {
var defaultValues = this.defaultObject,
retrievedValues;
if (storage) {
retrievedValues = JSON.parse(localStorage.getItem(this.dataName))
}
this.userFields.each(function () {
var currentKey = this.name,
value = ((storage) ? retrievedValues[currentKey] : this.value) || defaultValues[currentKey];
$("#" + currentKey + "Txt").text(value);
$("#" + currentKey).val(value)
});
ns.bind = function (selector) {
return new FormSave(selector)
}
};
try {
uid = new Date();
(storage = window.localStorage).setItem(uid, uid

Code Snippets

(function (ns, $) {
  "use strict"; //prevents "bad practice"

  //variables needed for localStorage test
  var storage, fail, uid;

  //For flexibility, it would be best if your code 
  //wasn't build for a specific form only
  //instead, allow it to create instances and attach
  //to different forms with different default data
  function FormSave(selector, defaultValues) {
    this.defaultValues = defaultValues || {};
    this.dataName = selector;
    this.form = $(selector);
    this.userFields = $('.formFieldUserData', this.form);
  }

  //our save function
  FormSave.prototype.save = function () {

    var userData = {};

    //using the "early return" pattern to avoid further
    //code exectution if condition fails, and avoid
    //deep indentation
    if (!storage) {
      return;
    }

    //since each() assigns the current DOM element in
    //the collection as this, we can get name and value
    //directly from the DOM element, saving you function calls
    this.userFields.each(function () {
      userData[this.name] = this.value;
    });

    //set the items
    localStorage.setItem(this.dataName, JSON.stringify(userData));
  };

  //our setting function
  FormSave.prototype.set = function () {

    //cache the default values into this scope
    var defaultValues = this.defaultObject,
      retrievedValues;

    //if local storage is present, get and parse
    if (storage) {
      retrievedValues = JSON.parse(localStorage.getItem(this.dataName));
    }

    this.userFields.each(function () {

      //cache the current key
      var currentKey = this.name,
        //then the value to be used is:
        //if storage is present, use either the retrieved values or the default
        //if not, then use the present value or the default
        value = ((storage) ? retrievedValues[currentKey]: this.value) || defaultValues[currentKey];

      //for safe text insertion, use text() since it escapes the value first
      $("#" + currentKey + "Txt").text(value);
      $("#" + currentKey).val(value);
    });

    //our easy binding function exposed to our FormSave namespace
    ns.bind = function (selector) {
      return new FormSave(selector);
    };
  };

  //a quick test for localstorage
  //from: http://mathiasbynens.be/notes/localstorage-pattern
  try {
    uid = new Date();
    (storage = window.localStorage).setItem(uid, uid);
    fail = storage.getItem(uid) !== uid;
    storage.removeItem(uid);
    fail && (storage = false);
  } catch (e) {}

}(this.FormSave = this.FormSave || {}, jQuery));

//usage:

//bind this functionality to a form id'ed "someForm" with
//the given default values
var someForm = FormSave.bind('#someForm', {
  name: 'John',
  email: 'john@example.com',
  phone: '9999999999',
  desc: 'some description'
});

//then with the given reference, call save and set
someForm.save();
someForm.set();
(function (ns, $) {
  "use strict";
  var storage, fail, uid;

  function FormSave(selector, defaultValues) {
    this.defaultValues = defaultValues || {};
    this.dataName = selector;
    this.form = $(selector);
    this.userFields = $('.formFieldUserData', this.form)
  }
  FormSave.prototype.save = function () {
    var userData = {};
    if (!storage) {
      return
    }
    this.userFields.each(function () {
      userData[this.name] = this.value
    });
    localStorage.setItem(this.dataName, JSON.stringify(userData))
  };
  FormSave.prototype.set = function () {
    var defaultValues = this.defaultObject,
      retrievedValues;
    if (storage) {
      retrievedValues = JSON.parse(localStorage.getItem(this.dataName))
    }
    this.userFields.each(function () {
      var currentKey = this.name,
        value = ((storage) ? retrievedValues[currentKey] : this.value) || defaultValues[currentKey];
      $("#" + currentKey + "Txt").text(value);
      $("#" + currentKey).val(value)
    });
    ns.bind = function (selector) {
      return new FormSave(selector)
    }
  };
  try {
    uid = new Date();
    (storage = window.localStorage).setItem(uid, uid);
    fail = storage.getItem(uid) !== uid;
    storage.removeItem(uid);
    fail && (storage = false)
  } catch (e) {}
}(this.FormSave = this.FormSave || {}, jQuery));

Context

StackExchange Code Review Q#23693, answer score: 3

Revisions (0)

No revisions yet.