patternjavascriptMinor
Binding events in constructor
Viewed 0 times
eventsconstructorbinding
Problem
I am trying to get familiar with OOP JavaScript, in particular working with the prototype pattern and would love some pointers/suggestions on how to improve my code.
I think I have understood the basics well, and have been able create class instances successfully.
I am however, a little unsure on how to work with events when it comes to representing these instances with a UI. Until now, I have been binding my events in the constructor before the element gets added to the DOM, but am sure this could be improved:
Ideally, I'd like to be able to build up a relatively complex UI of 'people', with various controls on each person to perform actions related to that person only. Apologies for the contrived example, I'm just trying to get the concept right in my head before doing anything more real-world.
I've also included a working jsfiddle incase this is easier to work with: http://jsfiddle.net/z59q2rab/.
I think I have understood the basics well, and have been able create class instances successfully.
I am however, a little unsure on how to work with events when it comes to representing these instances with a UI. Until now, I have been binding my events in the constructor before the element gets added to the DOM, but am sure this could be improved:
function Person(data) {
var self = this;
self.data = data;
self.$element = $('' +
'' + data.name + '' +
'' + data.age + '' +
'');
// bind events
self.$element.find('.name').click(function () {
self.speakName();
});
self.$element.find('.age').click(function () {
self.speakAge();
});
// render element
$('.person-container').append(self.$element);
}
Person.prototype.speakName = function () {
alert(this.data.name);
};
Person.prototype.speakAge = function () {
alert(this.data.age);
};Ideally, I'd like to be able to build up a relatively complex UI of 'people', with various controls on each person to perform actions related to that person only. Apologies for the contrived example, I'm just trying to get the concept right in my head before doing anything more real-world.
I've also included a working jsfiddle incase this is easier to work with: http://jsfiddle.net/z59q2rab/.
Solution
It feels like you have muddled the model, view, and controller. With a name like
A second potential problem is that concatenating strings into HTML without escaping can lead to JavaScript injection vulnerabilities (also known as cross-site scripting). I recommend using
border: 1px solid black;
margin-bottom: 5px;
}
`
Person, I would expect this object to act as a model. Constructing a person shouldn't also make its UI. At the least, make the UI rendering a separate explicit step.A second potential problem is that concatenating strings into HTML without escaping can lead to JavaScript injection vulnerabilities (also known as cross-site scripting). I recommend using
$.text() to fill in the text.function Person(data) {
this.name = data.name;
this.age = data.age;
}
Person.prototype.speakName = function() {
alert(this.name);
};
Person.prototype.speakAge = function() {
alert(this.age);
};
Person.prototype.render = function($container) {
var $element = $(
''
);
// Fill text and bind events
var self = this;
$element.find('.name')
.text(self.name)
.click(function () {
self.speakName();
});
$element.find('.age')
.text(self.age)
.click(function () {
self.speakAge();
});
// render element
$container.append($element);
};
//////////////////////////////////////////////////////////////////////
var persons = [
new Person({
name: 'Tom',
age: 30
}),
new Person({
name: 'Dick',
age: 25
}),
new Person({
name: 'Harry',
age: 45
})
];
for (var i = 0; i
.person {border: 1px solid black;
margin-bottom: 5px;
}
`
Context
StackExchange Code Review Q#80686, answer score: 4
Revisions (0)
No revisions yet.