Chowające się menu Opcje

[WiruSSS]

Witam

Na wstępie chcialbym zaznaczyć, że stworzenie tego nie nastręcza mi problemów. Bardziej nie daje mi spokoju czy aby nie robię tego jakoś naokoło, nieoptymalnie. Być może istnieje jakiś wygodniejszy nieoczywisty dla mnie sposób?

http://jsfiddle.net/WiruSSS/zPXUK/

[PrinceOfPersia]

Dla jasności jeszcze raz napiszę: Czy da się owy efekt uzyskać w prostszy sposób

na pewno da się skrócić kod. To co mi się rzuca w oczy, to straszne ilości zduplikowanego kodu:

var menu = $(this).children(),
direction = ($(this).position().left > 0 ? 'right' : 'left'),
anim = {};

to masz z 3 razy w różnych funkcjach powtórzone.

tak samo to:

menu.find('li:' + (direction === 'left' ? 'first' : 'last') + '-child').hide();
menu.find('li:' + (direction === 'left' ? 'last' : 'first') + '-child').show();
menu.animate(anim)

a powtarzanie sie jest bez sensu, bo będziesz chciał coś zmienić - 3 razy więcej roboty.
W zasadzie dobrze byłoby wydzielić powtarzający się kod do jakiejś osobnej funkcji pomocniczej, np.:

Kod

function getMenuParams(self) {
  var menu = $(self).children(), direction = ($(self).position().left > 0 ? 'right' : 'left'), anim = {};
  return {menu:menu, direction:direction, anim:anim};
}

a potem zamiast:

Kod

var menu = $(this).children(),
            direction = ($(this).position().left > 0 ? 'right' : 'left'),
            anim = {};

wywołujesz tylko funkcję i przypisujesz zmienne z funkcji:

Kod

var p = getMenuParams(this);
var menu = p.menu, direction = p.direction, anim = p.anim;

Czy muszę rozgraniczać się do zdarzenia chowania menu osobno a do zdarzenia wysuwania menu osobno?

Nie musisz. Możesz zrobić to w jednej funkcji. Na dobrą sprawę showMenu i hideMenu robią dokładnie to samo, różnią się tylko jedną linijką:
w hide jest

Kod

anim[direction] = -menu.width() + 'px';

a w show

Kod

anim[direction] = 0;

czyli możesz zamiast zrobić osobne funkcję dać jedną funkcję showHideMenu, i showHideMenu(true) oznacza pokazanie, showHideMenu schowanie:

Kod

    showHideMenu: function (isShowing) {
        "use strict";
                var p = getMenuParams(this);
                var direction = p.direction, anim = p.anim, menu = p.menu;

        anim[direction] = isShowing? 0 : -menu.width() + 'px';

        menu.find('li:' + (direction === 'left' ? 'first' : 'last') + '-child').hide();
        menu.find('li:' + (direction === 'left' ? 'last' : 'first') + '-child').show();
        menu.animate(anim);
    },

coś jak to (nie chce mi się tego testować czy wszystko się odpala, ale w zarysie tak mniej więcej).

No i żeby zrobić "lukier składniowy", możesz zostawić funkcje showMenu / hideMenu, ale w ten sposób, że będą odwoływać się do showHideMenu:

Kod

showMenu: function() {
  this.showHideMenu(true);
},

hideMenu: function() {
  this.showHideMenu(false);
},

tym sposobem dalej będziesz mógł pisać hideMenu() / showMenu().

EDIT:
albo zrób tak jak kolega napisał, z utworzeniem konstruktora i osobnego obiektu do każdego menu. Whatever works (IMG:style_emoticons/default/wink.gif) Ważne, żeby nie powtarzać zbyt dużej ilości kodu (bo tak jak teraz masz jest tragicznie pod kątem powtarzania). Zresztą wsadzenie czegoś w jeden obiekt wcale nie przeczy temu co napisałem.

Chociaż i tego rzadko używam bo w programowaniu obowiązuje zasada - deklaruje zmienne jak najbliżej miejsca w którym zostaną użyte.

w JavaScript do zalecanych praktyk należy deklarowanie zmiennych na początku funkcji - chyba wiesz dlaczego (dla postronnych - chodzi o to, że zasięg zmiennych deklarowanych przez 'var' jest na całą funkcję (a nie na blok kodu, jak to jest np. w C++)

- nie CSS decyduje o tym po której stronie ma być slider. Tu mieszasz reprezentację z logiką. To skrypt ma decydować po której stronie ma być slider.

to po której stronie/w którym miejscu ma być slider, to właśnie warstwa reprezentacji i powinno to być ustawiane w HTML/CSS.