jqueryで繰り返す処理の時に配列をリセットしたい

Question

複数table内にあるdl配下のdtの高さを取得し、そのtable内での最大値で揃えたい

下記のようにtable内にdlを3つ配置し、そのdtの高さを最大値で揃えようとしています。
tableが複数あるためeach functionでそれぞれのtable内のdtの高さを配列に入れ最大値を取得、.cssでdtの高さを設定しようとしたのですが
配列がtableごとでリセットされず意図しないheightになってしまいます。
初心者のため見苦しいコードですが、ご助言いただけますと幸いです。

発生している問題・エラーメッセージ

配列がリセットされず高さが足されていってしまいます。

該当のソースコード

<div class="wrapper">
<table class="parent">
    <tbody>
        <tr>
            <td>
                <dl>
                    <dt class="child">テキスト</dt>
                    <dd></dd>
                    <dd></dd>
                    <dd></dd>
                </dl>
                <dl>
                    <dt class="child">テキスト</dt>
                    <dd></dd>
                    <dd></dd>
                    <dd></dd>
                </dl>
                <dl>
                    <dt class="child">テキスト</dt>
                    <dd></dd>
                    <dd></dd>
                    <dd></dd>
                </dl>
            </td>
        </tr>
    </tbody>
</table>
<table class="parent">
    <tbody>
        <tr>
            <td>
                <dl>
                    <dt class="child">テキスト</dt>
                    <dd></dd>
                    <dd></dd>
                    <dd></dd>
                </dl>
                <dl>
                    <dt class="child">テキスト</dt>
                    <dd></dd>
                    <dd></dd>
                    <dd></dd>
                </dl>
                <dl>
                    <dt class="child">テキスト</dt>
                    <dd></dd>
                    <dd></dd>
                    <dd></dd>
                </dl>
            </td>
        </tr>
    </tbody>
</table>
<table class="parent">
    <tbody>
        <tr>
            <td>
                <dl>
                    <dt class="child">テキスト</dt>
                    <dd></dd>
                    <dd></dd>
                    <dd></dd>
                </dl>
                <dl>
                    <dt class="child">テキスト</dt>
                    <dd></dd>
                    <dd></dd>
                    <dd></dd>
                </dl>
                <dl>
                    <dt class="child">テキスト</dt>
                    <dd></dd>
                    <dd></dd>
                    <dd></dd>
                </dl>
            </td>
        </tr>
    </tbody>
</table>
</div>

$(function () {
    $('.parent').each(function () {
        var array = [];
        for (var i = 0; i < $('.child', this).length; i++) {
            var dt_height = $('.child', this).outerHeight();
            array.push(dt_height);
            var maxH = Math.max.apply(null, array);
            $('.child').css({
                "height": maxH + "px"
            });
        }
    });
});

試したこと

array.length = 0;
var array = [];
を思い当たる箇所に挿入してみましたが想定している動作にはなりませんでした。

Answer 1

配列の初期化はvar array = []やarray.length = 0;で正しく行えます。
代入を伴う方法は参照が剥がれてしまうので、引数等で持ってきた元の配列に副作用を与えたいニッチな要件ではarray.length = 0;を使う事がありますが、基本的にあんまりよろしい方法ではなく使わないで済むにこしたことはありません。

今回は配列の初期化が原因ではないようです。
こういった勘違いで明後日の方向を向いてしまい大ハマリしてしまう事は私もよくやるので、
今回の質問はかなりベストなタイミングだったと思います。

---

ではコードを読んでいきます。

配列がtableごとでリセットされず意図しないheightになってしまいます。

本当？これ確認した？
こうすればarrayの中身がデベロッパーツール(F12キー)のconsoleで表示されるから確認してみて。

$(function () {
    $('.parent').each(function () {
        var array = [];
        for (var i = 0; i < $('.child', this).length; i++) {
            var dt_height = $('.child', this).outerHeight();
            array.push(dt_height);
            var maxH = Math.max.apply(null, array);
            $('.child').css({
                "height": maxH + "px"
            });

            // arrayの中身を確かめる為にconsole上に表示する
            console.log(array);
        }
    });
});

私の机上デバッグだと正しく初期化されて、要素数1、2、3を繰り返すだけに見えるからね。

$('.parent').each(function () {

ふむふむ…

for (var i = 0; i < $('.child', this).length; i++) {

は？なんで急に書き方が変わるの？
each使えばthisが上書きされるから困ったんだと思うけど、
childの中身は普通に考えればchild内で全て解決出来るはずだから、改めて$(this)すれば大抵は事足りるはずだよ。

ループというのは小さい範囲で描いたほうがなにかと作りやすく不具合も出にくい。
外に出せるものは外に出そう！

$('.child').css({

いや、今まで第二引数にthis入れて$('.child', this)にしてたよね？
なんで急にthis削ったの？
今回の不具合はここが一番怪しい、修正すれば動くと思うよ。

var dt_height = $('.child', this).outerHeight();

【追記】合計値が足される問題が片付いても動かないバグみっけた
for (i = 0; i < length; i++)の形式でループさせてるけど、
その変数iはどこにも使われていない。

複数要素該当する場合、outerHeightは最初の要素の縦幅を返すそうだよ。
リファレンスのouterHeightの第一行目にバッチリとfor the first elementと記載されている。

jQueryだと$('.child', this).eq(i)みたいな感じでi番目のchildを取り出す事を明示しないとダメ。
リファレンスのeqに英語だけど使い方が載ってるよ。

---

という訳でこれらを修正すれば動くようになるでしょう。
しかし、全体的にコードの書き方が怪しいのでリファクタリングしましょう。

他に挙げるとすれば、
Math.maxを通った結果は配列の最大値にきまっているので変数名にするまでもありません。
また、jQueryのeachメソッドは戻り値を捨てるので配列を予め宣言する必要がありますが、
jQueryのmapを使えば宣言と配列生成を同時に行えそうですね。

このへんを反映するとこのようなコードになります。

$(function () {
    $('.parent').each(function () {
        var $child = $('.child', this); // 二度使うので変数を宣言してキャッシュ取る
        var heights = $child.map(function () {
            return $(this).outerHeight();
        }).get();
        $child.css({
            height: Math.max.apply(null, heights) + "px"
        });
    });
});

Comments

[unison_t]

ご回答及び丁寧な解説ありがとうございます！！
ご提示いただいたコードで無事に目的を果たせました。

ループや配列について自分がほぼ無知に近いと再認識しました。
こんなにシンプルに綺麗に書けるようになりたいです。。

とてもわかりやすかったのでじっくり読ませていただいて勉強させていただきます！

[unison_t]

追加でさらに詳しく解説していただき感謝です！
ループ、配列でつまづいているのでとても助かります。

[unison_t]

追記ありがとうございます！
これまさに悩んでいた箇所の一つです。
しっかり理解せず雰囲気でコードを書いてるのが丸わかりで恥ずかしい限りです。。
不明だったところが全てクリアになり、感動しています！！

Answer 2

高さを設定する処理の部分で「すべてのchildクラスを持った要素」に適用しているからだと思います。

$('.child').css({
    "height": maxH + "px"
});

他の部分ではちゃんと、「(ループで)現在のテーブル配下でchildクラスを持った要素」という指定になっています。$('.child', this)の部分です。

Comments

[unison_t]

コメント遅れました、ありがとうございます！
他の方にもご指摘いただきましたように$('.child', this)の部分でおかしくなっていたようです。

[NozomuIkuta]

解決したようでよかったです