I am working on a personal project for managing my bookmarks, which is basically a web page to manage my bookmarks by categories. Managing here means everything - adding, viewing, updating, searching by categories, adding new categories. As can be guessed there is need of AJAX and JavaScript. I have a JavaScript file for managing this interactivity on the webpage. I am a newbie when it comes to JS/jQuery/AJAX so I have doubts about the quality of the code that I have written. I decided to get part of it reviewed. I got an answer due to which I decided to refactor the whole thing. I am placing both files here for review.The first file is largely based on triggers. The second one has functions instead of using triggers. I want to know whether there is any improvement or not in any aspect. Or is the old one better in any aspect. Any suggestions are ok. I am not willing to lose loose coupling for anything as it makes it a lot harder for me to reason about the flow and make changes.If someone wants to see complete code for project then it is here. If anyone wants to see the below code as gists then they are here.PRE//Variables for URLsvar URL_CATEGORY_AUTO = auto/category/;var URL_BOOKMARK_AUTO = auto/bookmark/;var URL_PAGE_OPEN = open/;var URL_BOOKMARK_LIST = bookmark/category/;var URL_BOOKMARK_BY_NAME = bookmark/name/;var URL_CATEGORY = category/;//Variables for selector stringsvar CLASS_CATEGORY = .category;var CLASS_DEL_CATEGORY = .delete-cat;var CLASS_BOOKMARK = .bookmark;var CLASS_UI_MENU_ITEM = .ui-menu-item;//Variables for getting length of classesvar LEN_DEL_CATEGORY = delete-cat .length;var LEN_BOOK_CATEGORY = col-md-12 bookmark .length;//Variables for selectorsvar CATEGORY_INPUT = $(#category_inp);var BOOKMARK_LIST = $(#bookmarks-list);var CATEGORY_LIST_SEARCH = $(#category_list_search);var CATEGORY_LIST_ADD = $(#category_list_add);var BOOKMARK_NAME = $(#bookmark-name);var CATEGORY_BOX = $(#category-box);var TOP_WRAPPER = $(#top-wrapper);var ADD_BOOKMARKS = $(#add-bookmark);//Variables for event namesvar EV_ADD_CATEGORY = addCategory;var EV_CLICK = click;var EV_UPDATE_BOOKMARKS = updateBookmarks;var EV_CLEAR_ALL = clearAll;//Auxiliary functionsfunction valueInSelector(valueOf, selector) {    var result = false;    $(selector).each(function () {        if (valueOf.val() == $.trim($(this).text())) {            result = true;        }    });    return result;}function getClassString(curObj, start) {    return $(curObj).attr('class').substr(start);}function bindEvents(curObj, e) {    var triggerName = ;    if (e.keyCode == 13) {        triggerName = enterKey;    } else if (e.altKey && e.keyCode == N.charCodeAt(0)) {        triggerName = clickAltN;    }    $(curObj).trigger(triggerName);}//Action for body Load$(function () {    TOP_WRAPPER.toggle();    CATEGORY_INPUT.autocomplete({        source: function (req, res) {            $.getJSON(URL_CATEGORY_AUTO, { excludes: getAllCategories(), term: CATEGORY_INPUT.val() },                res);        }    });    CATEGORY_BOX.autocomplete({        source: function (req, res) {            $.getJSON(URL_CATEGORY_AUTO, { excludes: getAllCategories(), term: CATEGORY_BOX.val() },                res);        }    });    BOOKMARK_NAME.autocomplete({        source: URL_BOOKMARK_AUTO    });});//Auxiliary functions for categoriesfunction getAllCategories() {    var classIds = [];    $(CLASS_DEL_CATEGORY).each(function () {        classIds.push(getClassString(this, 11));    });    return classIds.join(,);}function testCategory(categoryInp) {    if (valueInSelector(categoryInp, CLASS_UI_MENU_ITEM) == false) {        alert(Not a category);    } else if (valueInSelector(categoryInp, CLASS_CATEGORY) == true) {        alert(Category already added);        categoryInp.val();    } else {        return true;    }    return false;}function addCategory(curObj, categoryList, triggerData) {    $.ajax({        url: URL_CATEGORY,        data: {'value': $(curObj).val()},        success: function (output) {            categoryList.append(output);            $(curObj).val();            if (triggerData.trigger_obj !== undefined) {                triggerData.trigger_obj.trigger(triggerData.next_trig);            }        }    });}function deleteCategory(curObj) {    var categoryId = getClassString(curObj, LEN_DEL_CATEGORY);    $(CLASS_CATEGORY + . + categoryId).remove();}//Actions common to both category boxes$(#category_inp,#category-box).on({    keyup: function (e) {        bindEvents(this, e);    },    enterKey: function () {        if (testCategory($(this))) {            $(this).trigger(EV_ADD_CATEGORY);        }    },    clearAll: function () {        $(this).val();    }});//Actions for Search's category boxCATEGORY_INPUT.on({    addCategory: function () {        addCategory(this, CATEGORY_LIST_SEARCH, {            trigger_obj: BOOKMARK_LIST,            next_trig: EV_UPDATE_BOOKMARKS        });    }});//Actions for Add Bookmark's category boxCATEGORY_BOX.on({    addCategory: function () {        addCategory(this, CATEGORY_LIST_ADD, {});    }});CATEGORY_LIST_SEARCH.on({    clearAll: function () {        $(this).html();        BOOKMARK_LIST.trigger(EV_CLEAR_ALL);    }});CATEGORY_LIST_ADD.on({    clearAll: function () {        $(this).html();    }});//Actions for delete of Category - SearchCATEGORY_LIST_SEARCH.on(EV_CLICK, CLASS_DEL_CATEGORY, function () {    deleteCategory(this);    BOOKMARK_LIST.trigger(EV_UPDATE_BOOKMARKS);});//Actions for delete of Category - AddCATEGORY_LIST_ADD.on(EV_CLICK, CLASS_DEL_CATEGORY, function () {    deleteCategory(this);});//Actions for bookmark ListBOOKMARK_LIST.on({    updateBookmarks: function () {        $.ajax({            url: URL_BOOKMARK_LIST,            data: {'ids': getAllCategories()},            success: function (output) {                BOOKMARK_LIST.html(output);            }        });    },    clearAll: function () {        $(this).html();    }});//Actions for BookmarkBOOKMARK_LIST.on(EV_CLICK, CLASS_BOOKMARK, function () {    $.ajax({        url: URL_PAGE_OPEN,        data: {'id': getClassString(this, LEN_BOOK_CATEGORY)}    });});//Actions for Show/Hide Bookmark addition screenADD_BOOKMARKS.on({    click: function () {        TOP_WRAPPER.toggle();        CATEGORY_INPUT.trigger(EV_CLEAR_ALL);        CATEGORY_LIST_SEARCH.trigger(EV_CLEAR_ALL);    }});//Action for BookMark Name - WIPBOOKMARK_NAME.on({    keyup: function (e) {        bindEvents(this, e);    },    enterKey: function () {        $.ajax({            url: URL_BOOKMARK_BY_NAME,            data: {'name': BOOKMARK_NAME.val()},            success: function (output) {                for (var category in output) {                    CATEGORY_BOX.trigger(EV_ADD_CATEGORY);                }            }        })    }});POST//Variables for URLsvar URL_CATEGORY_AUTO = auto/category/;var URL_BOOKMARK_AUTO = auto/bookmark/;var URL_PAGE_OPEN = open/;var URL_BOOKMARK_LIST = bookmark/category/;var URL_BOOKMARK_BY_NAME = bookmark/name/;var URL_CATEGORY = category/;//Variables for selector stringsvar CLASS_CATEGORY = .category;var CLASS_DEL_CATEGORY = .delete-cat;var CLASS_BOOKMARK = .bookmark;var CLASS_UI_MENU_ITEM = .ui-menu-item;//Variables for getting length of classesvar LEN_DEL_CATEGORY = delete-cat .length;var LEN_BOOK_CATEGORY = col-md-12 bookmark .length;//Auxiliary Functionsfunction valueInSelector(valueOf, selector) {    var result = false;    $(selector).each(function () {        if (valueOf.val() == $.trim($(this).text())) {            result = true;        }    });    return result;}function getClassString(curObj, start) {    return $(curObj).attr('class').substr(start);}function initSelector(uniqueId, customId) {    var selector = $(uniqueId);    selector.attr('customId', customId);    return selector;}//Variables for selector Stringsvar strCategoryInput = 'CATEGORY_INPUT';var strCategoryListSearch = 'CATEGORY_LIST_SEARCH';var strTopWrapper = 'TOP_WRAPPER';var strBookmarkName = 'BOOKMARK_NAME';var strCategoryBox = 'CATEGORY_BOX';var strCategoryListAdd = 'CATEGORY_LIST_ADD';var strAddBookmarks = 'ADD_BOOKMARKS';var strBookmarkList = 'BOOKMARK_LIST';//Variables for selectorsvar CATEGORY_INPUT = initSelector(#category_inp, strCategoryInput);var CATEGORY_LIST_SEARCH = initSelector(#category_list_search, strCategoryListSearch);var TOP_WRAPPER = initSelector(#top-wrapper, strTopWrapper);var BOOKMARK_NAME = initSelector(#bookmark-name, strBookmarkName);var CATEGORY_BOX = initSelector(#category-box, strCategoryBox);var CATEGORY_LIST_ADD = initSelector(#category_list_add, strCategoryBox);var ADD_BOOKMARKS = initSelector(#add-bookmark, strAddBookmarks);var BOOKMARK_LIST = initSelector(#bookmarks-list, strBookmarkList);//Controller functions - STARTfunction eventClear(curObj) {    var customId = curObj.attr(customId);    switch (customId) {        case strCategoryInput:        case strCategoryBox:            curObj.val();            break;        case strCategoryListSearch:            eventClear(BOOKMARK_LIST);        case strCategoryListAdd:        case strBookmarkList:            curObj.html();            break;        default:            alert(eventClear for  + customId +  Not Bound);    }}function eventEnter(e, curObj) {    var customId = curObj.attr(customId);    switch (customId) {        case strCategoryInput:        case strCategoryBox:            if (testCategory(curObj)) {                eventAddCategory(curObj);            }            break;        default:            alert(eventEnter for  + customId +  Not Bound);    }}function eventAddCategory(curObj) {    var customId = curObj.attr(customId);    switch (customId) {        case strCategoryInput:            addCategory(curObj, CATEGORY_LIST_SEARCH, updateBookmarks);            break;        case strCategoryBox:            addCategory(curObj, CATEGORY_LIST_ADD, null);            break;        default:            alert(eventAddCategory for  + customId +  Not Bound);    }}function eventClick(e, curObj) {    var customId = curObj.attr(customId);    switch (customId) {        case strAddBookmarks:            bindEvents(e, ADD_BOOKMARKS);            TOP_WRAPPER.toggle();            eventClear(CATEGORY_INPUT);            eventClear(CATEGORY_LIST_SEARCH);            break;        default:            alert(eventClick for  + customId +  Not Bound);    }}function bindEvents(e, curObj) {    if (e.keyCode == 13) {        eventEnter(e, curObj);    } else if (e.altKey && e.keyCode == N.charCodeAt(0)) {    }}$(function () {    TOP_WRAPPER.hide();    CATEGORY_INPUT.autocomplete({        source: function (req, res) {            $.getJSON(URL_CATEGORY_AUTO, { excludes: getAllCategories(), term: CATEGORY_INPUT.val() },                res);        }    });    CATEGORY_BOX.autocomplete({        source: function (req, res) {            $.getJSON(URL_CATEGORY_AUTO, { excludes: getAllCategories(), term: CATEGORY_BOX.val() },                res);        }    });    BOOKMARK_NAME.autocomplete({        source: URL_BOOKMARK_AUTO    });});//Auxiliary functions for Categories - STARTfunction getAllCategories() {    var classIds = [];    $(CLASS_DEL_CATEGORY).each(function () {        classIds.push(getClassString(this, 11));    });    return classIds.join(,);}function testCategory(categoryInp) {    if (valueInSelector(categoryInp, CLASS_UI_MENU_ITEM) == false) {        alert(Not a category);    } else if (valueInSelector(categoryInp, CLASS_CATEGORY) == true) {        alert(Category already added);        categoryInp.val();    } else {        return true;    }    return false;}function addCategory(curObj, list_update, func) {    $.ajax({        url: URL_CATEGORY,        data: {'value': $(curObj).val()},        success: function (output) {            list_update.append(output);            $(curObj).val();            if (func !== null) {                func();            }        }    });}function deleteCategory(curObj) {    var categoryId = getClassString(curObj, LEN_DEL_CATEGORY);    $(CLASS_CATEGORY + . + categoryId).remove();}//Auxiliary functions for Bookmarksfunction updateBookmarks() {    $.ajax({        url: URL_BOOKMARK_LIST,        data: {'ids': getAllCategories()},        success: function (output) {            BOOKMARK_LIST.html(output);        }    });}//jQuery for binding events and delegating events START$(#category_inp,#category-box,#bookmark-name,#add-bookmark).on(keyup, function (e) {        bindEvents(e, $(this));    });$(#add-bookmark).on(click, function (e) {        eventClick(e, $(this));    });//TODO Find a way to refactor this event delegation properly//Actions for delete of Category - SearchCATEGORY_LIST_SEARCH.on(click, CLASS_DEL_CATEGORY, function (e) {    deleteCategory(this);    updateBookmarks();});//Actions for delete of Category - AddCATEGORY_LIST_ADD.on(click, CLASS_DEL_CATEGORY, function (e) {    deleteCategory(this);});//Actions for BookmarkBOOKMARK_LIST.on(click, CLASS_BOOKMARK, function (e) {    $.ajax({        url: URL_PAGE_OPEN,        data: {'id': getClassString(this, LEN_BOOK_CATEGORY)}    });});
Personal project for managing my bookmarks
javascript;jquery
The GoodNaming conventions seem to be consistentThe programming pattern seems to be consistentBasically, the code just looks consistently written. That being said, it's not really organized.The BadAll variables are globalAll variables appear to be constantsAll functions are globalAll the code put together performs a mishmash of jobsThe UglyNothing is really ugly here. For that you'd need a helluva lot more Clint Eastwood.Breaking Down Your Application Into ComponentsTo really organize your code, you need to break it down into components that focus on one specific task, for example, adding a category. Looking at your code, I can see the following tasks:Showing all categoriesShowing bookmarks in a categoryAdding a categoryEditing a categoryRemoving a categoryAdding a bookmarkEditing a bookmarkRemoving a bookmarkYou can break your application down into several controllers:CategoryIndexController: Shows a list of all categoriesBookmarkIndexController: Shows the bookmarksCategoryController: Adds, edits and removes categoriesBookmarkController: Adds, edits and removes bookmarksWhen you change anything with categories, you need to refresh the category list. Same thing with bookmarks. For this you can use events. Controllers would publish an event, say category:updated. The CategoryIndexController would subscribe to category:updated and refresh the category list. Same idea with BookmarkIndexController.Now the AJAX functionality is currently all global. I usually go for the Repository Pattern to encapsulate AJAX calls. For that you would have two more classes:CategoryRepository: Provides methods for the basic CRUD operations on categoriesBookmarkRepository: Provides methods for the basic CRUD operations on bookmarksThis way all AJAX is centralized and abstracted away. None of the rest of the application even needs to know AJAX is at work. You could rewrite your repository layer to use asynchronous calls to the browser's IndexedDB for an offline application and you wouldn't need to refactor any of your other code.ControllersOver the years I keep coming back to this basic pattern when creating controllers in JavaScript.Controllers:Handle all the user interaction for a certain root element in the document on inwardsUtilize the repository objects to modify dataPublish events on a common event bus when something interesting happens within the controller that the outside world might want to know aboutSubscribe to events in order to respond to events that other controllers publishSome pseudo code outlining this pattern is below:BookmarkIndexControllerfunction BookmarkIndexController(element) {    this.element = typeof element === string ? document.getElementById(element) : element;    this.document = element.ownerDocument;    this.$document  $(this.document);    this.$element = $(element);    this.handleBookmarkUpdated = this.handleBookmarkUpdated.bind(this);    this.bookmarks = new BookmarkRepository();}BookmarkIndexController.prototype = {    bookmarks: null,    element: null,    $element: null,    constructor: BookmarkIndexController,    init: function() {        this.$document.on(bookmark:updated, this.handleBookmarkUpdated);        this.bookmarks.findAll()            .done(function(html) {                this.$element.html(html);            }.bind(this);    },    handleBookmarkUpdated: function(event) {        this.bookmarks.findAll()            .done(function(html) {                this.$element.html(html);            }.bind(this);    }}BookmarkRepositoryfunction BookmarkRepository() {}BookmarkRepository.prototype = {    baseUrl: /bookmarks,    findAll: function() {        return $.ajax({            type: GET,            url: this.baseUrl + /index        });    }}And some HTML to kick things off:<div id=bookmarks-list></div><script type=text/javascript>    var bookmarkIndex = new BookmarkIndexController(bookmarks-list)    bookmarkIndex.init();</script>Separating things into these layers makes them testable. You could write Jasmine or Mocha tests for your controller and repository layer:describe(BookmarkIndexController, function() {    var element, controller, repository, promise;    beforeEach(function() {        element = document.createElement(div);        repository = new BookmarkRepository();        controller = new BookmarkIndexController(element);        controller.repository = repository;        promise = {            done: function(callback) {                this.callback = callback;            }        };    });    it(loads bookmarks, function() {        spyOn(repository, findAll).and.returnValue(promise);        spyOn(promise, done).and.callThrough();        controller.init();        // Mock AJAX request        promise.callback(Bookmarks go here!);        expect(promise.done).toHaveBeenCalled();        expect(element.innerHTML).toBe(Bookmarks go here!);    });});I know this is a lot to throw at a code review, but this does allow for loose coupling, plus all the functionality is componentized and reusable. Since each class has all of its dependencies as public properties (like the repository) it becomes easy to mock up those dependencies for the purpose of unit testing.