2017-02-27 21 views
0

私は自分のjavascriptを練習しています。非表示の段落を表示するリンクを作成しました。このコードでは現在2 'のforループを使用しています。私はどうにかして 'for'ループの関数を作成してから、関数を再利用するべきでしょうか?段落のリストを初期化するときトグル(「隠す」)以来リファクタリングfor javascript 'for'ループ

var paragraphs = document.getElementsByTagName('p'), 
    firstParagraph = paragraphs[0], 
    link = document.createElement('a'); 
link.innerHTML = 'Show more'; 
link.setAttribute('class', 'link'); 
link.setAttribute('href', '#'); 
firstParagraph.appendChild(link); 

for (var i = 1; i <= paragraphs.length - 1; i++) { 
    paragraphs[i].classList.add('hide') 
} 

function toggleHide(e) { 
    e.preventDefault; 
    var paragraphs = document.getElementsByTagName('p'); 
    for (i = 1; i <= paragraphs.length - 1; i++) { 
     paragraphs[i].classList.toggle('hide'); 
    } 
} 

link.addEventListener('click', toggleHide) 
+0

これは価値のあるものになります。この場合は特に若干異なることがあるからです。 –

+0

意見の問題はありません(私はそう思わない)というのは、あなたのコードを読みやすくフォーマットしてインデントしなければならないということです。 :-) –

答えて

1

も追加(「隠す」)の同じことを行うだろう、一つの関数に重複したコードをプルアップする良いです。例えば

var paragraphs = document.getElementsByTagName('p'), 
firstParagraph = paragraphs[0], 
link = document.createElement('a'); 
link.innerHTML = 'Show more'; 
link.setAttribute('class' , 'link'); 
link.setAttribute('href' , '#'); 
firstParagraph.appendChild(link); 
toggleHideAll(); 

function toggleHide(e){ 
    e.preventDefault; 
    var paragraphs = document.getElementsByTagName('p'); 
    toggleHideAll(); 
} 

function toggleHideAll(){ 
    for(i = 1 ; i <= paragraphs.length-1 ; i++){ 
     paragraphs[i].classList.toggle('hide'); 
    } 
} 

link.addEventListener('click' , toggleHide) 
+0

あなたの説明のために多くのありがとう。多くのお礼ありがとうございます – James

1

@Solmonが言うようにはい、両端を達成するために、単一のループが、良いでしょう:

function toggleHideAll(){ 
    for (var i = 1; i <= paragraphs.length-1; i++) { 
     paragraphs[i].classList.toggle('hide'); 
    } 
} 

これを表現するために、より慣用的な方法がありますしかし、元のフォームは、標準形式に慣れている開発者に混乱しているので、私はあなたにそれを使用することをお勧めします:

ループ変数が 未満長さであるが、ループは、ゼロから始まるある

(ない以下長さから1を引いたものに等しいです。この場合、ループは元のものとまったく同じことをしません。なぜなら、オリジナルは実際に最初の段落をスキップするからです。それが意図的なら、むしろループのパラメータを微調整するよりも、私は段落のすべてをトグルして、ループの外側のコードの行を持つ特殊なケースの取り扱いをお勧めします。また

function toggleHideAll() { 
    for (var i = 0; i < paragraphs.length; i++) { 
     paragraphs[i].classList.toggle('hide'); 
    } 
    paragraphs[0].classList.remove('hide'); 
} 

を、それはあなたが避けることができたときは本当にうれしいですあなたのコードの明示的なループは、全体として:

function toggleHideAll() { 
    paragraphs.forEach(p => p.classList.toggle('hide')); 
    paragraphs[0].classList.remove('hide'); 
} 
+0

説明と時間がありがとうございました。 – James