patternjavascriptMinor
(Swedish) Calendar of the year 2015 using Firebase
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',
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
as
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
could be
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
You are using
You are using variables
I thought this part was funny:
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 applicationcould 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 applicationvar 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.