patternjavascriptMinor
Having a parent respond to a child's event
Viewed 0 times
respondhavingparentchildevent
Problem
I have a parent object (a display window) and it has a few child objects (buttons and controls on the window). I would like one child object to hide itself when another is clicked. Currently, I have my parent object bind to a custom event expected to be fired from the child object. Then, the parent object passes the command on to its other child.
Example:
I have two questions / concerns about this code:
Example:
//Namespace for the 'Orders' page.
$(function Orders() {
"use strict";
var orders = $('#Orders');
var ordersSearchResult = new OrdersSearchResult();
var ordersSearchDisplay = new OrdersSearchDisplay();
var ordersEditDisplay = new OrdersEditDisplay();
ordersSearchResult.selector.bind('searchResultLinkClicked', function () {
ordersEditDisplay.hide();
ordersSearchDisplay.fadeIn();
});
});I have two questions / concerns about this code:
- I expose a property called 'selector' for
ordersSearchResult. This is a direct reference to the jQuery DOM element. As I understand it this is bad practice -- objects should expose methods to affect their DOM elements, but not the DOM elements themselves. Previously, I passed theonSearchResultLinkClickedanonymous function into a method ofordersSearchResultwhich bound the event to the selector internally. I felt that that implementation was counter-intuitive, though, as it did not read very clearly to see anonymous functions being passed into 'binding' methods.
ordersEditDisplayandordersSearchDisplayexpose methodshide()andfadeIn()which affect their DOM elements, butordersEditDisplay/ordersSearchDisplayare not DOM elements themselves. Is it bad practice to 'confuse' other developers like this? The code could read:ordersEditDisplay.selector.hide()but I did not want to expose the selector publically.
Solution
It's really hard to judge if exposing
Also, it depends on your architecture design. If you're using a Model View Controller approach, then yes. Elements associated with the view will be exposed.
Check out some of the examples for Backbone.js.
Here are some other factors that come into play:
The code you provided looks great. As long as you produce consistent readable code that has a good level of data abstraction and works, then you shouldn't worry about the small details.
If you're still concerned about it though then just create an adapter for the
Old Code:
New Code:
Here are a few additional tips.
1) Name variables and properties based on their overall purpose and type.
Programmer A: There's a problem with orders in your code.
Programmer B: Which one? Are you talking about the function, jQuery object, element id, or something else?
To avoid this situation, create unique names assuming the language is case insensitive.
Old code:
New code:
More info here: "How to name variables"
2) Add the dollar sign as a prefix for jQuery variables.
It would make more sense to rename
3) Try not to create method names that are the same from a different library API.
Instead of
4) Use the Singleton Pattern or an object literal to create a namespace.
Great examples can be found here
Final Code:
selector is the "right" thing to do without reviewing more of your source code.Also, it depends on your architecture design. If you're using a Model View Controller approach, then yes. Elements associated with the view will be exposed.
Check out some of the examples for Backbone.js.
Here are some other factors that come into play:
- Coding standards
- size of your source
- how strictly you're required to follow Object Oriented Programming, OOP.
- who will maintain your code.
The code you provided looks great. As long as you produce consistent readable code that has a good level of data abstraction and works, then you shouldn't worry about the small details.
If you're still concerned about it though then just create an adapter for the
.bind().Old Code:
ordersSearchResult.selector.bind('searchResultLinkClicked', function () {
ordersEditDisplay.hide();
ordersSearchDisplay.fadeIn();
});New Code:
...
OrdersSearchResult.prototype.bind = function(eventName, fn ){
this.selector.bind( eventName, fn );
};
...
var ordersSearchResult = new OrdersSearchResult();
ordersSearchResult.bind('searchResultLinkClicked', function () {
ordersEditDisplay.hide();
ordersSearchDisplay.fadeIn();
});Here are a few additional tips.
1) Name variables and properties based on their overall purpose and type.
Programmer A: There's a problem with orders in your code.
Programmer B: Which one? Are you talking about the function, jQuery object, element id, or something else?
To avoid this situation, create unique names assuming the language is case insensitive.
Old code:
$(function Orders() {
"use strict";
var orders = $('#Orders');
...New code:
$(function OrdersFunc() {
"use strict";
var $orders = $('#OrdersTable');
...More info here: "How to name variables"
2) Add the dollar sign as a prefix for jQuery variables.
It would make more sense to rename
selector as $el.3) Try not to create method names that are the same from a different library API.
ordersEditDisplay.hide() sounds like ordersEditDisplay returns a jQuery object, but that isn't the case.Instead of
ordersEditDisplay.hide();, try ordersEditDisplay.hideView();4) Use the Singleton Pattern or an object literal to create a namespace.
Great examples can be found here
Final Code:
var mySite = {};
mySite.Orders = {};
mySite.Orders.init = function() {
"use strict";
var $orders = $('#OrdersTable');
var ordersSearchResult = new OrdersSearchResult();
var ordersSearchDisplay = new OrdersSearchDisplay();
var ordersEditDisplay = new OrdersEditDisplay();
ordersSearchResult.$el.bind('searchResultLinkClicked', function () {
ordersEditDisplay.hideView();
ordersSearchDisplay.fadeInView();
});
};
$(mySite.Orders.init);Code Snippets
ordersSearchResult.selector.bind('searchResultLinkClicked', function () {
ordersEditDisplay.hide();
ordersSearchDisplay.fadeIn();
});...
OrdersSearchResult.prototype.bind = function(eventName, fn ){
this.selector.bind( eventName, fn );
};
...
var ordersSearchResult = new OrdersSearchResult();
ordersSearchResult.bind('searchResultLinkClicked', function () {
ordersEditDisplay.hide();
ordersSearchDisplay.fadeIn();
});$(function Orders() {
"use strict";
var orders = $('#Orders');
...$(function OrdersFunc() {
"use strict";
var $orders = $('#OrdersTable');
...var mySite = {};
mySite.Orders = {};
mySite.Orders.init = function() {
"use strict";
var $orders = $('#OrdersTable');
var ordersSearchResult = new OrdersSearchResult();
var ordersSearchDisplay = new OrdersSearchDisplay();
var ordersEditDisplay = new OrdersEditDisplay();
ordersSearchResult.$el.bind('searchResultLinkClicked', function () {
ordersEditDisplay.hideView();
ordersSearchDisplay.fadeInView();
});
};
$(mySite.Orders.init);Context
StackExchange Code Review Q#15986, answer score: 3
Revisions (0)
No revisions yet.