patternjavascriptMinor
Web files browser
Viewed 0 times
browserfilesweb
Problem
I wrote a web files browser. I want to refactor my JavaScript code, but I'm new to JavaScript.
I have some thoughts:
-
Split the file into several files:
Do you have any other ideas?
-
Naming conventions: How should functions\vars names be written?
-
I wrote my code flat. How can I refactor it to objects easily?
```
var folders_tree;
var data_per_folder = [];
var current_folder = {};
// folder_id: "",
// folder_name: ""
//};
var current_page = 1;
var file_types_lookup = [];
$(function () {
fillFileTypesLookup();
// set topbar_links onClick to alter div content
$(".nav li a").attr({
"onclick": "topbarItem_OnClick(this.href)"
});
// set pagination onClick to alter div file_content
$(".pagination li a").attr({
"onclick": "paginationItem_OnClick(this)"
});
$("#add_folder").click(function () {
$('#new_folder_modal').modal('show');
});
$("#remove_folder").click(function () {
sendDeleteFolderRequestAndUpdateFilesTree();
});
$("#edit_folder").click(function () {
$('#edit_folder_name').val(current_folder.attr('name'));
$('#edit_folder_color').val(current_folder.attr('color'));
$('#edit_folder_description').val(current_folder.attr('description'));
$('#edit_folder_modal').modal('show');
});
//otherwise will create modal with default values.
$('#extra_details_modal').modal({
backdrop: true,
keyboard: true
});
//otherwise will create modal with default values.
$('#new_folder_modal').modal({
backdrop: true,
keyboard: true
});
//set jsTree
$("#jstree").jstree({
"json_data": {
//initial - demo only, usually takes json from the controller
//static data, or function(node, mappingBeforeRequestToserver)
//"data": data,
//comb
I have some thoughts:
-
Split the file into several files:
FoldersManager.js
FilesManager.js
Paginator.js
GlobalVars.js
Do you have any other ideas?
-
Naming conventions: How should functions\vars names be written?
-
I wrote my code flat. How can I refactor it to objects easily?
```
var folders_tree;
var data_per_folder = [];
var current_folder = {};
// folder_id: "",
// folder_name: ""
//};
var current_page = 1;
var file_types_lookup = [];
$(function () {
fillFileTypesLookup();
// set topbar_links onClick to alter div content
$(".nav li a").attr({
"onclick": "topbarItem_OnClick(this.href)"
});
// set pagination onClick to alter div file_content
$(".pagination li a").attr({
"onclick": "paginationItem_OnClick(this)"
});
$("#add_folder").click(function () {
$('#new_folder_modal').modal('show');
});
$("#remove_folder").click(function () {
sendDeleteFolderRequestAndUpdateFilesTree();
});
$("#edit_folder").click(function () {
$('#edit_folder_name').val(current_folder.attr('name'));
$('#edit_folder_color').val(current_folder.attr('color'));
$('#edit_folder_description').val(current_folder.attr('description'));
$('#edit_folder_modal').modal('show');
});
//otherwise will create modal with default values.
$('#extra_details_modal').modal({
backdrop: true,
keyboard: true
});
//otherwise will create modal with default values.
$('#new_folder_modal').modal({
backdrop: true,
keyboard: true
});
//set jsTree
$("#jstree").jstree({
"json_data": {
//initial - demo only, usually takes json from the controller
//static data, or function(node, mappingBeforeRequestToserver)
//"data": data,
//comb
Solution
A couple notes on design:
-
Split the code up into different objects that handle specific tasks. You don't need to split your code up into different files, because it's not that long (yet), but you can split the functionality into objects, like this:
-
Put code that manipulates the DOM into a
-
As ericW mentioned, don't use global variables. They can conflict with global variables from another file which could result in hard-to-find bugs. You can stick them inside the new objects using the following pattern:
And some minor things:
-
Why are you doing things like this:
You can use the same
-
This:
can be simplified to:
See http://james.padolsey.com/javascript/truthy-falsey/ for more info.
-
Split the code up into different objects that handle specific tasks. You don't need to split your code up into different files, because it's not that long (yet), but you can split the functionality into objects, like this:
FoldersManager = {
init: function() {
$("#add_folder").click(function () {
$('#new_folder_modal').modal('show');
});
...
}
};-
Put code that manipulates the DOM into a
$.ready() block, which guarantees that the DOM will be ready when your code executes:$.ready(function() {
FoldersManager.init();
FilesManager.init();
Paginator.init();
});-
As ericW mentioned, don't use global variables. They can conflict with global variables from another file which could result in hard-to-find bugs. You can stick them inside the new objects using the following pattern:
$.ready(function() {
FoldersManager.init();
FilesManager.init();
Paginator.init();
});
FoldersManager = (function() {
var folders_tree;
var data_per_folder = [];
var current_folder = {};
return {
init: function() {
$("#add_folder").click(FoldersManager.openAddFolderDialog);
...
},
openAddFolderDialog: function() {
$('#new_folder_modal').modal('show');
}
};
});And some minor things:
-
Why are you doing things like this:
$(".nav li a").attr({
"onclick": "topbarItem_OnClick(this.href)"
});You can use the same
$.click() function you were using for the anonymous functions:$(".nav li a").click(function() {
// or change the function to use the href attribute directly
// instead of taking it as a param
topbarItem_OnClick($(this).attr('href'));
});-
This:
if (data_per_folder[current_folder.attr('id')] == undefined)can be simplified to:
if (!data_per_folder[current_folder.attr('id')])See http://james.padolsey.com/javascript/truthy-falsey/ for more info.
Code Snippets
FoldersManager = {
init: function() {
$("#add_folder").click(function () {
$('#new_folder_modal').modal('show');
});
...
}
};$.ready(function() {
FoldersManager.init();
FilesManager.init();
Paginator.init();
});$.ready(function() {
FoldersManager.init();
FilesManager.init();
Paginator.init();
});
FoldersManager = (function() {
var folders_tree;
var data_per_folder = [];
var current_folder = {};
return {
init: function() {
$("#add_folder").click(FoldersManager.openAddFolderDialog);
...
},
openAddFolderDialog: function() {
$('#new_folder_modal').modal('show');
}
};
});$(".nav li a").attr({
"onclick": "topbarItem_OnClick(this.href)"
});$(".nav li a").click(function() {
// or change the function to use the href attribute directly
// instead of taking it as a param
topbarItem_OnClick($(this).attr('href'));
});Context
StackExchange Code Review Q#7551, answer score: 2
Revisions (0)
No revisions yet.