Skip to main content
replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

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 listarguments 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.

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.

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.

Source Link
Brythan
  • 7k
  • 3
  • 22
  • 37

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.