Personal project for managing my bookmarks

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 URLs
var 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 strings
var 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 classes
var LEN_DEL_CATEGORY = "delete-cat ".length;
var LEN_BOOK_CATEGORY = "col-md-12 bookmark ".length;

//Variables for selectors
var 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 =

The Good

• Naming conventions seem to be consistent

• The programming pattern seems to be consistent

Basically, the code just looks consistently written. That being said, it's not really organized.

The Bad

• All variables are global

• All variables appear to be "constants"

• All functions are global

• All the code put together performs a mishmash of jobs

The Ugly

Nothing is really ugly here. For that you'd need a helluva lot more Clint Eastwood.

Breaking Down Your Application Into Components

To 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 categories

• Showing bookmarks in a category

• Adding a category

• Editing a category

• Removing a category

• Adding a bookmark

• Editing a bookmark

• Removing a bookmark

You can break your application down into several "controllers":

• CategoryIndexController: Shows a list of all categories

• BookmarkIndexController: Shows the bookmarks

• CategoryController: Adds, edits and removes categories

• BookmarkController: Adds, edits and removes bookmarks

When 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 categories

• BookmarkRepository: Provides methods for the basic CRUD operations on bookmarks

This 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.

Controllers

Over 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 inwards

• Utilize the repository objects to modify data

• Publish events on a common event bus when something interesting happens within the controller that the outside world might want to know about

• Subscribe to events in order to respond to events that other controllers publish

Some pseudo code outlining this pattern is below:

BookmarkIndexController

function 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);
    }
}

BookmarkRepository

function BookmarkRepository() {
}
BookmarkRepository.prototype = {

    baseUrl: "/bookmarks",

    findAll: function() {
        return $.ajax({
            type: "GET",
            url: this.baseUrl + "/index"
        });
    }
}

And some HTML to kick things off:

    var bookmarkIndex = new BookmarkIndexController("bookmarks-list")
    bookmarkIndex.init();

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).toHaveBee

StackExchange Code Review Q#53896, answer score: 6