HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

iPhone notes application replica using HTML/CSS

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
applicationiphonecssreplicanotesusinghtml

Problem

I've written a web application that is a replica of the iPhone's notes app. It turned out quite well, you can do everything you can on the iPhone's app except for sharing and sending it via texts etc. I've been sitting here trying to figure out a way to DRY this up, so far I think I've done a pretty good job in doing so.

However, there is one thing that annoys me a bit and I haven't been able to come up with a better solution then what I have now. It's these parts:

ctrl.functions.hideBtn('#done-btn');
ctrl.functions.hideBtn('#all-notes');
ctrl.functions.hideBtn('#edit');
ctrl.functions.showBtn('#add-note');


I repeat myself a lot by doing these things.. And I do them like 4-5 times throughout the code whenever the view needs to change. What I tried to do in the showBtn and hideBtn is sending in an array containing all the buttons that needed to be manipulated and then using a loop in the respective functions to loop over that array and hide/show them. This, however, didn't work for some reason and after thinking about it it didn't really feel like a better solution, it just got more messy.

How can I remove this repetitiveness?

Also, feel free to leave feedback on anything else that can be improved. I'll leave the full code down below as well as two pictures of what it looks like for those who are interested.

```
$(document).ready(function() {

var model = {
ajaxURLs: {
addNote: '../includes/notepages/addnote.php',
editNote: '../includes/notepages/editnote.php',
deleteNote: '../includes/notepages/deletenote.php',
allNotes: '../includes/notepages/allnotes.php',
},
wrappers: {
topBar: $('#top-bar'),
content: $('#content-wrapper'),
timestamp: undefined
}
}

var ctrl = {

bindElements: (function() {
model.wrappers.topBar.on('click', '#add-note', function() {

var promise = ctrl.functions.restCall(model.ajaxURLs.addNote);
promise.done(funct

Solution

Are you sure that you need hideBtn? You forget to use it once and say

$('#add-note').hide();


instead. But that's considerably shorter than

ctrl.functions.hideBtn('#add-note');


Your hideBtn and showBtn functions don't seem to actually provide you anything that you are currently using. You could just as well leave them as hide() and show(). That leaves the problematic pattern, but at least it's not as verbose.

The usual fix for this pattern would be to make a single hideAll with something like

hideAll: function() {
        $('#top-bar .btn').hide();
    },


Or just write:

$('#top-bar .btn').hide();


Instead of using a separate function at all.

And then for show, you can use the arguments list:

showBtns: function() {
        for ( var i = 0; i < arguments.length; i++ ) {
            $(arguments[i]).show();
        }
    },


Which you'd call like

ctrl.functions.showBtns('#all-notes', '#edit');
        ctrl.functions.showBtns('#done-btn');


However, you don't always specify the button states. I haven't tried to track through the pattern, so I don't know if the buttons you don't specify stay hidden or stay shown. In fact, I'm not sure that it is known. It seems like there would likely be multiple paths to any particular point, so there would be multiple possible button arrangements. If this is correct (i.e. you shouldn't know which buttons are shown other than the ones that you explicitly hide or show), then I'm not sure that you have a DRY situation. You have a number of similar but different situations that don't respond mechanically to being combined.

If your current pattern is correct, then I believe that you are overthinking this. Your solution is likely to be more verbose and fragile than the problem. You already increase verbosity and indirection with your hideBtn and showBtn functions. Perhaps it would be better to step back and make things simpler rather than more complicated.

Code Snippets

$('#add-note').hide();
ctrl.functions.hideBtn('#add-note');
hideAll: function() {
        $('#top-bar .btn').hide();
    },
$('#top-bar .btn').hide();
showBtns: function() {
        for ( var i = 0; i < arguments.length; i++ ) {
            $(arguments[i]).show();
        }
    },

Context

StackExchange Code Review Q#75850, answer score: 3

Revisions (0)

No revisions yet.