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

HTML menu with sub-menus

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

Problem

I have an HTML menu with sub-menus and also sub-sub-menus. This code verifies each menu's trigger. If clicked, sub-menu opens. Sub-menus also have triggers that open sub-sub-menus and so on. A working example can be found here. This menu is a modified version of the menu found here, made by Mary Lou. I want this to be my mobile menu but I don't want to manually enter all the cases.

My code is very lengthy and repetitive. How can I simplify it?

```
( function( window ) {
'use strict';

// http://stackoverflow.com/a/11381730/989439
function mobilecheck() {
var check = false;
(function(a){if(/(android|ipad|playbook|silk|bb\d+|meego).+mobile|avantgo|bada\/|blackberry|blazer|compal|elaine|fennec|hiptop|iemobile|ip(hone|od)|iris|kindle|lge |maemo|midp|mmp|netfront|opera m(ob|in)i|palm( os)?|phone|p(ixi|re)\/|plucker|pocket|psp|series(4|6)0|symbian|treo|up\.(browser|link)|vodafone|wap|windows (ce|phone)|xda|xiino/i.test(a)||/1207|6310|6590|3gso|4thp|50[1-6]i|770s|802s|a wa|abac|ac(er|oo|s\-)|ai(ko|rn)|al(av|ca|co)|amoi|an(ex|ny|yw)|aptu|ar(ch|go)|as(te|us)|attw|au(di|\-m|r |s )|avan|be(ck|ll|nq)|bi(lb|rd)|bl(ac|az)|br(e|v)w|bumb|bw\-(n|u)|c55\/|capi|ccwa|cdm\-|cell|chtm|cldc|cmd\-|co(mp|nd)|craw|da(it|ll|ng)|dbte|dc\-s|devi|dica|dmob|do(c|p)o|ds(12|\-d)|el(49|ai)|em(l2|ul)|er(ic|k0)|esl8|ez([4-7]0|os|wa|ze)|fetc|fly(\-|_)|g1 u|g560|gene|gf\-5|g\-mo|go(\.w|od)|gr(ad|un)|haie|hcit|hd\-(m|p|t)|hei\-|hi(pt|ta)|hp( i|ip)|hs\-c|ht(c(\-| |_|a|g|p|s|t)|tp)|hu(aw|tc)|i\-(20|go|ma)|i230|iac( |\-|\/)|ibro|idea|ig01|ikom|im1k|inno|ipaq|iris|ja(t|v)a|jbro|jemu|jigs|kddi|keji|kgt( |\/)|klon|kpt |kwc\-|kyo(c|k)|le(no|xi)|lg( g|\/(k|l|u)|50|54|\-[a-w])|libw|lynx|m1\-w|m3ga|m50\/|ma(te|ui|xo)|mc(01|21|ca)|m\-cr|me(rc|ri)|mi(o8|oa|ts)|mmef|mo(01|02|bi|de|do|t(\-| |o|v)|zz)|mt(50|p1|v )|mwbp|mywa|n10[0-2]|n20[2-3]|n30(0|2)|n50(0|2|5)|n7(0(0|1)|10)|ne((c|m)\-|on|tf|wf|wg|wt)|nok(6|i)|nzph|o2im|op(ti|wv)|oran|owg1|p800|pan(a|d|t)|pdxg|pg(13|\-([1-8]|c))|phil|pire

Solution

One thing that I saw right away, while I was editing your question to make it more noticeable, is that you have 3 functions that are the same, they just have different names.

function gnMenu( el, options ) {
    this.el = el;
    this._init();
}

function gnSubMenu( el, options ) {
    this.el = el;
    this._init();
}

function gnSubMenu2( el, options ) {
    this.el = el;
    this._init();
}


You don't need to do this. It looks like they all take in the same parameters and do the same thing, so just use one function for the 3 of them.

function menu( el, options ) {
    this.el = el;
    this._init();
}


One step to being DRYer already (DRY = Don't Repeat Yourself)

Code Snippets

function gnMenu( el, options ) {
    this.el = el;
    this._init();
}

function gnSubMenu( el, options ) {
    this.el = el;
    this._init();
}

function gnSubMenu2( el, options ) {
    this.el = el;
    this._init();
}
function menu( el, options ) {
    this.el = el;
    this._init();
}

Context

StackExchange Code Review Q#29579, answer score: 2

Revisions (0)

No revisions yet.