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

Avoiding nested conditionals and comparing strings

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

Problem

I have a web page with a set of icons that are grey by default:

If a user clicks one, it toggles green (.toggleClass('green')):

If a user clicks a green one, it toggles back to grey (.removeClass('green')).

Only one icon can be green at any given time, so if you have a green icon and you click a grey icon, the icon that was originally green will change to grey while the one you clicked would change to green.

Each of these icons corresponds to a section on the page.

-
If all the icons are grey, all the sections on the page are visible.

-
If one icon is green, only that icon's section is visible, the sections represented by the grey icons are hidden.

Here is the code that controls that behavior:

$(document).on 'click', '.toggler', ->
  icons = [$('#toggle-videos'),$('#toggle-images'),$('#toggle-words')]
  sections = [$('#videos'),$('#images'),$('#words')]
  if $(this).hasClass('green')
    $(this).toggleClass('green')
    i = 0
    while i < sections.length
      sections[i].show()
      i++
  else
    $(this).addClass('green')
    i = 0
    while i < icons.length
      if icons[i][0] != $(this)[0]
        icons[i].removeClass('green')
        sections[i].hide()
      else      
        j = 0
        while j < sections.length
          if $(this).attr('id').indexOf(sections[j].attr('id')) != -1
            sections[j].show()
          j++
      i++


I feel it is very hard to understand what this code does simply by reading it. I feel I am missing a simpler way to accomplish the following:

  • Associate an icon with a specific section on the page



  • Keep the green state of an icon associated with the visibility of a specific section on the page



My primary concerns with the code:

  • I have several nested if statements with nested while statements



  • I am checking the relationship between a section and an icon by checking if the icon.id has part of the string (this is the icon element and j is a counter):


`if $(this).attr('id').indexOf(sectio

Solution

I would propose to use data attributes. E.g. icons have data-section-id attribute.

if $(this).hasClass('green') // all should become greynow
{
    $('div.section').show(); // show all sections
    $(this).removeClass('green');
}
else // we turn one sectio on
{
    $('div.section').hide(); // show all sections
    $('#'+ $(this).data('sectionId')).show(); // show a corresponding section
    $('li.icon').removeClass('green'); // turn all icons off
    $(this).addClass('green'); // turn current one on
}


But of course, that requires some changes in HTML structure.

Code Snippets

if $(this).hasClass('green') // all should become greynow
{
    $('div.section').show(); // show all sections
    $(this).removeClass('green');
}
else // we turn one sectio on
{
    $('div.section').hide(); // show all sections
    $('#'+ $(this).data('sectionId')).show(); // show a corresponding section
    $('li.icon').removeClass('green'); // turn all icons off
    $(this).addClass('green'); // turn current one on
}

Context

StackExchange Code Review Q#46262, answer score: 2

Revisions (0)

No revisions yet.