patternMinor
Avoiding nested conditionals and comparing strings
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 (
If a user clicks a green one, it toggles back to grey (
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:
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:
My primary concerns with the code:
`if $(this).attr('id').indexOf(sectio
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.idhas part of the string (thisis the icon element andjis a counter):
`if $(this).attr('id').indexOf(sectio
Solution
I would propose to use data attributes. E.g. icons have data-section-id attribute.
But of course, that requires some changes in HTML structure.
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.