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

(Swedish) Calendar of the year 2015 using Firebase

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

Problem

I've made a calendar for the year of 2015 using Firebase for real-time updates when something changes in it. I'm quite happy with it but I'm interested in getting some feedback on my Javascripting regarding best-practices, what to avoid and anything in general that I can improve in this code. Even the tiniest of things is important so throw it at me. Also let me know what I did well so that I know it for future projects!

Unfortunately I can't include a JSfiddle or the actual site since I don't want random people to spam my database. Hope you understand.

```
//The database reference
var myDataRef = new Firebase('https://event-calender.firebaseio.com/events');

myDataRef.on('child_added', function(snapshot) {
var info = snapshot.val();
calendar.showAddedEvent(info.eventType, info.eventTime, info.eventLocation, info.eventAddress, info.eventDay);
});

var theParent;
var container = document.getElementById('container');

//An object containing any raw data
var dataModel = {

months: ['January', 'February', 'Marsch', 'April', 'May', 'June',
'July', 'August', 'September', 'October', 'November', 'December'
],
daysInMonth: [31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31],
years: ['2015'],
buttonTexts: ['Add', 'Delete'],

//All the red days of the year of 2015 (in Sweden)
redDays: ['1Jan', '4Jan', '6Jan', '11Jan', '18Jan', '25Jan',
'1Feb', '8Feb', '15Feb', '22Feb',
'1Mar', '8Mar', '15Mar', '22Mar', '29Mar',
'3Apr', '5Apr', '6Apr', '12Apr', '19Apr', '26Apr',
'1May', '3May', '10May', '14May', '17May', '24May', '31May',
'6Jun', '7Jun', '14Jun', '20Jun', '21Jun', '28Jun',
'5Jul', '12Jul', '19Jul', '26Jul',
'2Aug', '9Aug', '16Aug', '23Aug', '30Aug',
'6Sep', '13Sep', '20Sep', '27Sep',
'4Oct', '11Oct', '18Oct', '25Oct', '31Oct',
'1Nov', '8Nov', '15Nov', '22Nov', '29Nov',

Solution

From a once over;

I would write

//When a new child is added to the database, send a snapshot containing the data...
myDataRef.on('child_added', function(snapshot) { 
    var info = snapshot.val(); //And check the snapshot's data
calender.showAddedEvent(info.eventType, info.eventTime, info.eventLocation, info.eventAddress, info.eventDay); //And call the function to display it to the HTML
});


as

//Add new events to the calendar widget
myDataRef.on('child_added', function(snapshot) { 
    var event= snapshot.val(); 
    calender.showAddedEvent(event); 
});


The most important part are the comments, placing them on the right makes it harder to read. Also, they were not enlightening me. Your indenting was also off. The above is much clearer (to me). Also, since you call showAddedEvent only once in the whole code, and you pass only properties of event, it is easier to read/code/understand

var theParent; //Stores the current parent we're using
var container = document.getElementById('container'); //The container of the application


could be

var parent, //The current parent
    container = document.getElementById('container');


Again the comments were not too helpful, anybody could figure out after reading the variable name and id that the container was a container ;) Also it is considered better form to comma-chain your variable declarations.

I love dataModel, storing the month's ends is smart low tech.

You are using alert, that is considered bad form. Consider using (non-)modal forms.

You are using variables j and k, I am sure you could come up with something better.

I thought this part was funny:

//Prompts the user for a confirmation of the deletion of ALL events
var conf = confirm('You are about to delete all events, this cannot be undone. Are you sure you want to continue?');

switch (conf) {

    //If the answer is 'OK' go ahead with the procedure
    case true: calender.deleteAll();
               calender.removeEvents();
               break;

    //If the answer is 'Cancel' or 'X' the cancel the procedure
    case false: return; 
                break;

    //If something else happened during the request then alert an error message
    default: alert('There was an error processing your request');
             break;
}


confirm returns a boolean, I am not sure when default would ever be called ;)

Code Snippets

//When a new child is added to the database, send a snapshot containing the data...
myDataRef.on('child_added', function(snapshot) { 
    var info = snapshot.val(); //And check the snapshot's data
calender.showAddedEvent(info.eventType, info.eventTime, info.eventLocation, info.eventAddress, info.eventDay); //And call the function to display it to the HTML
});
//Add new events to the calendar widget
myDataRef.on('child_added', function(snapshot) { 
    var event= snapshot.val(); 
    calender.showAddedEvent(event); 
});
var theParent; //Stores the current parent we're using
var container = document.getElementById('container'); //The container of the application
var parent, //The current parent
    container = document.getElementById('container');
//Prompts the user for a confirmation of the deletion of ALL events
var conf = confirm('You are about to delete all events, this cannot be undone. Are you sure you want to continue?');

switch (conf) {

    //If the answer is 'OK' go ahead with the procedure
    case true: calender.deleteAll();
               calender.removeEvents();
               break;

    //If the answer is 'Cancel' or 'X' the cancel the procedure
    case false: return; 
                break;

    //If something else happened during the request then alert an error message
    default: alert('There was an error processing your request');
             break;
}

Context

StackExchange Code Review Q#68283, answer score: 2

Revisions (0)

No revisions yet.