Is this the best way of doing this. The sample code is a simple lookup of postnr (read zipcode). I'm trying to encapsulate all the client side logic inside one "class".
- Is this the way of doing callbacks? The this/that stuff sort of creeps me out a little
- Should the constructor just be a place where you declare variables? I have a init function call here that sets things up. But it is public and I wonder if there is some best practice around this?
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<title>Untitled Page</title>
<script type="text/javascript" src="http://ajax.aspnetcdn.com/ajax/jQuery/jquery-1.7.2.min.js"></script>
<script type="text/javascript">
$(document).ready(function () {
var controller = new ClientController("postnr", "poststed");
});
var ClientController = function (postNr, postSted) {
this.postNrCtrl = $("#" + postNr);
this.postStedCtrl = $("#" + postSted);
this.init();
};
ClientController.prototype = function () {
// private variables/methods
var init = function () {
// Keep reference to this
var that = this;
// Setup event handling
this.postNrCtrl.change(function () {
that.getPostSted($(this).val());
});
};
var getPostSted = function (postnr) {
// keep reference context/this
var that = this;
$.getJSON("http://fraktguide.bring.no/fraktguide/api/postalCode.json?country=no&pnr=" + postnr + "&callback=?", function (data) {
that.postStedCtrl.val(data.result);
});
};
// public variables/methods
return {
init: init,
getPostSted: getPostSted
};
} ();
</script>
</head>
<body>
<label for="postnr">Postnr</label> <input type="text" id="postnr" />
<label for="poststed">Poststed</label> <input type="text" id="poststed" />
</body>
</html>
1 Answer 1
Update: To answer your specific questions (which I just now realized I completely neglected):
The
this/thatstuff is one way of doing "context resolution" (you used the word "callback", but that's when you pass a function to another function). There are other ways, such asFunction.bind/jQuery.proxy, but in your case assigning a localthatvariable is a valid and customary way of doing it.Sure, you can have the constructor do nothing except declare and assign instance variables. But there are no written or unwritten rules regarding a construtor's responsibilities, so it's up to you what it should do. As you say the
initmethod ends up being public, which isn't terribly useful - in fact,initis only useful once in the object's lifetime, so it should probably be in the constructor. And if it's in the constructor, you don't need to assign instance variables; they work fine as local variables and closures. And if they're closures, you don't need thethis/thateither...
... and so you don't actually need a "class" anymore, which is how I arrived at the code below.
It depends on your specific needs, but with the example you give, it seems like overkill to have a "class" (constructor + prototype) to handle that bit of logic. You can make do with:
$(function () { // shorthand for $(document).ready(...)
var postNrCtrl = $("#postnr"),
postStedCtrl = $("#poststed");
postNrCtrl.on("change", function () {
var postnr = jQuery.trim(this.value);
// if the field is empty, clear the postStedCtrl and stop
if( !postnr ) {
postStedCtrl.val("");
return;
}
// Return if the value isn't 4 digits (i.e.
// if it isn't a valid Norwegian postal code)
if( !/^\d{4}$/.test(postnr) ) {
// perhaps you want to alert the user here before returning
return;
}
// Make an explicit JSONP request to get around same-origin policy
// jQuery takes care of encoding the params and adding the callback param
$.ajax({
type: "GET",
dataType: "jsonp",
url: "http://fraktguide.bring.no/fraktguide/api/postalCode.json",
data: {
country: "no",
pnr: postnr
}
}).done(function (data) {
if( data && data.valid ) { // check the response a little
postStedCtrl.val(data.result); // postStedCtrl is available via closure
}
});
});
});
On a more general note: For things like controller-logic, I find "classes" too cumbersome. You only need 1 instance of a controller, so coding a prototype and construtor only to instantiate it once, seems like overkill in a language like JavaScript that has object literals and closures.
-
\$\begingroup\$ "But there are no written or unwritten rules regarding a construtor's responsibilities" - I disagree. There is almost certainly an unwritten rule that a constructor should only ever prepare an object to be used. If there isn't, their should be - I'm having to debug code now that does lots of silly things in the constructor and it's a pain. \$\endgroup\$Dan– Dan2016年02月11日 09:02:51 +00:00Commented Feb 11, 2016 at 9:02
-
\$\begingroup\$ @DanPantry Fair point. My argument was a bit exaggerated. Of course you don't want your constructor to be a tangled mess; I was just rather presenting a counterpoint to the constructor being "just a place where you declare variables" as OP wrote. You're right that the constructor should only "prepare an object"; I just meant that that can involve more than just declaring vars. In OP's case preparation had been split into constructor and
initwhen there was really no need. \$\endgroup\$Flambino– Flambino2016年02月11日 10:21:07 +00:00Commented Feb 11, 2016 at 10:21