Is it considered a bad-practice to select distinct elements by catching their class/id in a string ?

Question

Hi, I've been using Jquery for some time now and I have a question concerning bad/good practices. Here is the deal. Lets say we have this HTML:

 <span class="а_s_trigger" id="tekst1">text1</span>
 <span class="а_s_trigger" id="video1">video1</span>
                <div class="a_s_songvideo" id="showvideo1">

                        <object>
                           //video1
                        </object>

                </div>
                <div class="a_s_songtext" id="showtext1">

                        <p>
                        //text1
                        </p>

                </div>

And the following jQuery function that triggers hide/show on click for an element that is either video or text.

$('.а_s_trigger')
        .bind('mouseover',function(e) {
       $(this).css({background:'#F3F3F3',cursor:'pointer'});
      })
      .bind('mouseout',function(e) {
       $(this).css({background:'#E6E6E6'}); 
      })
      .bind('click',function(e) {
       var id=$(this).attr('id');
       var status=$(this).attr('id').toString().slice(0,5);
       var index=$(this).attr('id').toString().slice(5,7);
       var visibility=$('#showtext'+index).css('display');

        if(status=='tekst1')
        {
         if(visibility=='block')
         {
          $('#showtext'+index).slideUp();
         }
         else if(visibility=='none')
         {
          $('#showtext'+index).slideDown();
         }

        }
        else if(status=="video")
        {
         $('#showvideo'+index).toggle();
        }




      });

Everything is working fine but what bugs me is the way I select the element I need :

var id=this.id;
var status=$(this).attr('id').toString().slice(0,5);
var index=$(this).attr('id').toString().slice(5,7);
var visibility=$('#showtext'+index).css('display');

Is it OK to assign variables to strings in such manner? I know that jQuery is all about selectors and the nice chaining functions like :

$(this).children().siblings().attr('id'); //and so on

Should I always try to select elements using the chain selectors or should I follow the principle "If it works don't change it!"?

Answer 1

It's fine. Since you're using the values multiple times in your conditional login, it makes sense to store them to a local variable first.

Random note: if you just need the id, this.id works just as well as $(this).attr('id').

Answer 2

I'm a Class man

 $(".button").  //etc

To me its much easier and you can grab multiple elements, but if you have no problem with your way as you said

If it works don't change it!"

Answer 3

I think conditionals make your code a bit cluttered and difficult to read. In my opinion you could encapsulate the status within the event handlers.

Add the status as classes to your elements.

<span class="а_s_trigger tekst" id="t_001">text1</span>
<span class="а_s_trigger video" id="v_001">video1</span>

Bind accordingly:

$('.а_s_trigger').bind('mouseover',function(e) {
                        $(this).css({background:'#F3F3F3',cursor:'pointer'});
                }).bind('mouseout',function(e) {
                        $(this).css({background:'#E6E6E6'});
                });

$('.а_s_trigger .tekst').bind('click',function(e) {
    var index=this.id.toString().slice(2,6);
    var $element = $('#showtext'+index);
    var visibility=$element.css('display');
    if(visibility=='block')
    {
        $element.slideUp();
    }
    else
    {
        $element.slideDown();
    }});

$('.а_s_trigger .video').bind('click',function(e) {
    var index=this.id.toString().slice(2,6);
    $('#showvideo'+index).toggle;
});

EDIT:

The text handler could be substituted by (with a tiny performance penalty):

$('.а_s_trigger .tekst').bind('click',function(e) {
    var index=this.id.toString().slice(2,6);
    $('#showtext'+index + ':visible').slideUp();
    $('#showtext'+index + ':hidden').slideDown();
});