It has been a few weeks (ok, a few months) since my last blog post on JavaScript design patterns. I'd apologize, but frankly, it will probably be a few more weeks until I blog on this subject again, so hopefully people aren't expecting a fast series here (grin). As a reminder, the idea behind this series is to create real, practical examples of various JavaScript design patterns based on the book, "Learning JavaScript Design Patterns" by Addy Osmani. (See my review here.) In this blog entry I'll be discussing the Revealing Module pattern.
Addy Osmani describes the Revealing Module pattern as:
The Revealing Module pattern came about as [Christian] Heilmann was frustrated with the fact that he had to repeat the name of the main object when he wanted to call one public method from another or access public variables. He also disliked the Module pattern's requirement of having to switch to object literal notation for the things he wished to make public.
There's two main issues here. First, the concept of "repeat the name of the main object when he wanted to call one public method from another..." If your only experience with the Module pattern is based on my previous blog post, then this may not make sense. Let's consider a simple example.
I've got a simple module here called revealModuleTest. (And yes, this isn't a "real world example", but I wanted to demonstrate the issue with a simple block first.) My module has one private method, and three public methods. The first one, doPriv, simply wraps a call to the private method. The second public method, pub1, just returns the number 2. Finally, testpub, just makes use of pub1 and multiplies it by two.
Note the three tests at the bottom. As the comments suggest, the final method will fail: Uncaught ReferenceError: pub1 is not defined
Why? When the result of the module (everything in that return) is returned back to the caller the scope has changed such that pub1, by itself, is no longer addressable from testpub. You won't have this issue if a private method calls another one, but you will definitely run into this with the public methods returned by your module. (I don't think I did a great job explaining this - I may come back and flesh this out.)
The fix is rather trivial - simply use the same API you would in your own code calling the module:
The second thing Heilmann talks about is the switch to using object notation for defining methods. Everything in the return block is using object notation. Personally, this doesn't seem like a big deal, but all things being considered, I would prefer to write more code in function x() syntax versus x:function().
If it sounds like the Revealing Module is just a cosmetic change, that would be fair I think. But don't simply dismiss it. Anything that makes you quicker, more efficient, etc in your development is probably a good thing. I'd also argue that the issue with "public calling public" is something you could easily accidentally trip into. If the Revealing Module pattern makes that easier to avoid, then it is yet another reason to consider it.
Let's look at an example of this - again - using my kinda stupid example.
As you can see, this isn't a radically changed version. It has the same smell, but is just structured a bit differently. Note that all of the real logic is in private functions. The return block is now far simpler - it just provides the public API.
Ok, so how about a real example? In my previous blog post I created a diary application that allowed you to create and read diary entries stored in WebSQL. I defined my Diary as a module. You can view that source here.
To modify this to match the Revealing Module pattern, I moved everything into private methods and created a much simpler return block.
So what do you think? I have to be honest. When I first read about this pattern, I really didn't think much about it. As I said, it seemed just cosmetic. But after seeing the change to my Diary module, it just... feels better.
I didn't put this up as a 'live' demo since the functionality is the exact same as before. (And as a reminder, it only works in WebKit browsers, which is bad, and will be addressed later.) If you really want to try this, you can download the code from the last entry and drop in the new diary. Since it is has the exact same API, everything will just plain work!
Archived Comments
I'm a little confused about the third code sample:
function testpub() {
return pub1() * 2;
}
return {
doPriv:priv1,
pub1:pub1,
testpub:testpub
}
Here, it looks like you've returned to the version of testpub that caused problems in the first code sample. Does it now automagically work because you switched to object notation in the return block?
When you use object notation in the return block, do you no longer need the technique used in the second code sample:
testpub:function() {
return revealModuleTest.pub1()*2;
}
?
"Here, it looks like you've returned to the version of testpub that caused problems in the first code sample. "
Technically yes, but the scope is now different. When I call myModule.testpub, the public interface, it is connected to the functions that were defined privately. testpub, pub1, and priv1. They can all see each other just fine.
I'm probably still not explaining that very well and I apologize. Hopefully the above is a bit clearer.
"When you use object notation in the return block, do you no longer need the technique used in the second code sample"
Correct.
Just hoist your module variable to the top and you can access it with no problem.
var module = {};
module.doIt = function(){
}
var b = function(){
module.doIt();
};
return module;
John, the fact that b has to prefix module to the call is part of the problem though. By moving to the revealing module pattern, all my functions can access each other 'naked'. It isn't that it isn't fixable, my second code sample shows that fix, it is more a, "If you prefer to use modules and want to not worry about it, consider this version."
(Imo)
Very good examples and I agree that it feels like a superior design convention. ( I prefer to think of simple patterns as design conventions.) I'm way far from being a JavaScript expert yet. Picking it up is slow going when when not using it for substantial client logic particularly because so much of it feels like "it just happens to work." And I hope you didn't put a damper on this saying this only works in WebKit. Obviously you mean your implementation with WebSQL. I'll be watching so I learn something about that too.
@Ray
I didn't see the code when I read it (RSS feed didn't include it) but I use that pattern often as well.
There is a gotcha though. If you run a test and want to do something like this, it'll fail:
//Jasmine test
diaryModule.callSomething();
expect(diaryModule.somethingElse).toHaveBeenCalled();
'somethingElse' is on the same module but the internal call doesn't call the module.
There may be a way around it but that bit me more than once. :)
@Gary: Yes - I meant the demo won't work outside of Webkit(Blink) cuz of WebSQL. It is part of my plan to bring up interfaces/etc as a way to support the same API but with different implementations.
@John: Interesting. Why would you write a test that sees if method X calls method Y? That seems... wrong. It seems like my unit tests shouldn't care *how* X does it's shit. It should just care that it has the right result. If i one day change X so that it doesn't call Y, but it still works right, my unit test test would break. That seems... wrong.
@Ray
Nah, it can be completely right. That is part of the unit. What you don't care about is what happens in 'diaryModule.somethingElse' but you could definitely care your method called the right other method.
Consider your method switching between an online or offline (local storage) service call or if param2 was passed in you want to make sure diaryModule.somethingElse received the right config object.
Your tests are there to test the unit, we agree, but your unit can include calling other methods; which you don't care what they do or return.
#havecode #willshare :-D
I don't know man. Consider your offline/online example. I've got a method called getData. When online, it gets the latest data. When offline, it gets data from cache. I suppose I could see you wanting to confirm it called the right method. But - if I'm offline and my method still returns data, then I'd consider it working fine as is. (This assumes a UT framework that could correctly mock network status.) To me, the crucial thing is, "Do I still work offline or online." The fact that I've got some private method called "getCrapFromCache" shouldn't be tied to my unit test of "getData". (imo)
In our case, we needed to make sure the proper service module was called, we mocked the service, and had it return data. Sure we could have just tested the data but in that case we're not confirming where the data came from: RestService or LocalService. (not actual names)
You have to know where your data comes from to verify a statement like this worked (dumbed down):
if(online) return RestService.getData();
else return LocalService.getData();
If you simply do:
expect(mymodule.getMyData()).toEqual(myMockArray);
...you're setting yourself up for failure when something changes in the logic controlling the 'online' variable.
You're still 100% testing your unit and nothing more but without confirming the method call your exposure to a problem is higher.
In this example, I wouldn't care if the method was called:
var b = function(){
var c = getData();
c.whatever = true;
return c;
}
//in my tests
var result = b();
expect(result).toEqual(myMockData);
expect(result.whatever).toBeTruthy();
I don't care where the data came from so long as it equals my mock data and 'whatever' is set properly.
"...you're setting yourself up for failure when something changes in the logic controlling the 'online' variable."
^ or your logic for determining whether to get online or offline data changes (ie - pull from cache if it exists).
I don't see how you can say, "I don't care where the data came from so long as it equals my mock data and 'whatever' is set properly.", and the say you check to see if a particular service is run. Those statements seem contradictory. If your intent is to ensure getData works online or offline, and you mock that status, then you expect data in both cases. You would test for getting data back, now how it got it.
"or your logic for determining whether to get online or offline data changes (ie - pull from cache if it exists)."
To me that means, one day I may decide if offline, we don't return data, but a flag of some sort. If so, that would mean a change to your unit tests. Your test for getData(while offline) should expect *no* data but the flag instead.
"I don't care where the data came from so long as it equals my mock data and 'whatever' is set properly."
^ That's related to the code right above it not my other point. The last bit of code is more to your point of not caring where the data comes from. It's a scenario where I would not test 'getData' was called. In other words, not contradictory at all as it had nothing to do with other 'switching' logic for online/offline.
Yes, if that logic changed, you'd write more tests and adjust your other test. What gets returned isn't a flat answer though so "no" data isn't valid in our case.
The overall point though is if you have logic in your functions that changes based on different conditions, you should verify those methods are called. Again, not a hard and fast rule but test coverage should be full units (meaning all functionality in that unit, not just the result).
Hmm. I see your point - just not sure I agree. :) But thanks - it has given me something to think about.
Cool beans and no prob. I always enjoy good discussions. :)
I really don't get the whole 'repeat the name of the main object when he wanted to call one public method from another' argument. Surely you can just use the 'this' pointer internally, or am I missing something?
testpub:function() {
return this.pub1()*2;
}
@james
'this' is evil and should be avoided at all times, IMHO. There are very rare instances where you need to use 'this'.
I can call any of your methods and change the context by simply doing: module.testpub.apply(myOtherModule). In this instance, myOtherModule is 'this' and 'this.pub1()' does not exist there.
Hi Raymond:
Would like to ask you something related to the pattern, when writing IFEs, the rule says that parenthesis at the beginning are not needed if the function is not assigned to a variable. Is there some reason to have parenthesis in this case?
Thanks so much,
Guillermo
I think I was just following the format I had seen. You are right though. (Sorry for the delay in responding.)
The Revealing Module Pattern has a major flaw when it comes to inheritance. See my answer on stack overflow -- http://stackoverflow.com/a/...
Thank's Raymond you have explained it very nicely,the part you are talking silly that helped me to understand why we need Revealing module pattern.