Mehrere Variablen bündeln

[borisbaer]

Hallo,
ich habe folgenden Code zum Filtern einer Tabelle:

function filterTable( e ) {
    let filter = e.target.value.toUpperCase();
    let tr = document.querySelector( 'tbody' ).rows;

    for ( let i = 0; i < tr.length; i++ ) {

        let td0 = tr[i].cells[0].textContent.toUpperCase();
        let td1 = tr[i].cells[1].textContent.toUpperCase();
        let td2 = tr[i].cells[2].textContent.toUpperCase();

        tr[i].style.display =
            td0.indexOf( filter ) > -1 ||
            td1.indexOf( filter ) > -1 ||
            td2.indexOf( filter ) > -1
            ? "" : "none";

    }
}

Wie ihr seht, werden die ersten drei Spalten durchsucht, doch dafür muss ich jedes Mal eine neue Variable definieren, auf die dann erneut jedes Mal der Filter angewandt werden muss. Ich habe mich gefragt, ob man das ganze auch irgendwie bündeln könnte?

Grüße
Boris

[JürgenB]

Hallo,

du kannst die Inhalte der TDs zu einem String verknüpfen und dann auf einmal durchsuchen.

Gruß
Jürgen

[Rolf B]

Hallo JürgenB,

Nein, nicht verknüpfen. Sonst findet sich ein Treffer vielleicht zellübergreifend.

Die Variablen sind temporär und gelten nur während einer Iteration, da muss man nichts verbessern.

Wenn man hinreichend gut Code lesen kann, kann man auch auf die Variablen verzichten und den indexOf direkt auf das Ergebnis des toUpperCase anwenden, also alles in der if Bedingung abhandeln. Dann gibt es keine Variablen mehr. Aber der Gewinn ist minimal, oder nicht vorhanden. Der just in time Compiler von JavaScript dürfte das wegoptimieren.

Rolf

[JürgenB]

Hallo Rolf,

Nein, nicht verknüpfen. Sonst findet sich ein Treffer vielleicht zellübergreifend.

ich habe den Code so verstanden, dass es egal ist, für welches Tabellenfeld der Filter greift also zellübergreifend gesucht werden soll. Nur die Zeile interessiert.

Gruß
Jürgen

[MudGuard]

Hi,

Nein, nicht verknüpfen. Sonst findet sich ein Treffer vielleicht zellübergreifend.

ich habe den Code so verstanden, dass es egal ist, für welches Tabellenfeld der Filter greift also zellübergreifend gesucht werden soll. Nur die Zeile interessiert.

egal in welcher Zelle der Begriff steht, ja.
Aber:
Gesucht werden soll nach "Wachstube".
Zelle 1 enthält "Rotes Wachs", Zelle 2 enthält "Tube mit Senf". Würde beim Originalcode keinen Treffer ergeben.
Sucht man dagegen in den verketteten Strings, also in "Rotes WachsTube mit Senf", erhält man einen Treffer.

Wenn, dann müßte man zwischen den Zell-Texten etwas unterbringen, das im Suchstring nicht vorkommen kann.

Also z.B: "@@@" -> dann würde man "Rotes Wachs@@@Tube mit Senf" nach "Wachstube" durchsuchen und keine Treffer bekommen.

cu,
Andreas a/k/a MudGuard

[JürgenB]

Hallo Andreas,

stimmt. Den Fall hätte ich erst beim Testen gesehen.

Gruß
Jürgen

[Gunnar Bittersmann]

@@borisbaer

function filterTable( e ) {

Wofür steht e?

    let filter = e.target.value.toUpperCase();

Ah, vermutlich für ein Event. Dann sollte der Parameter auch so benannt sein: event.

filter taucht im Folgenden nirgendwo rechts vom Gleichheitszeichen auf, d.h. es wird kein neuer Wert zugewiesen; es ist ein Konstante. let ist falsch, const ist richtig.

Gleiches gilt für tr, td0, td1 und td2.

Argl, tr. Der Bezeichnung ist nicht anzusehen, dass in der Konstanten(!) mehrere Zeilen (ein Array) gespeichert wird. Nicht

    let tr = document.querySelector( 'tbody' ).rows;

sondern

    const rows = document.querySelector( 'tbody' ).rows;

---

    for ( let i = 0; i < tr.length; i++ ) {

        let td0 = tr[i].cells[0].textContent.toUpperCase();
        ⋮
    }

Die 2000er haben angerufen. Sie hätten gerne ihre Old-school-Schleifen zurück.

Mit der erwähnten Umbenennung in rows:

    for ( let row of rows ) {

        const td0 = row.cells[0].textContent.toUpperCase();
        ⋮
    }

Wofür steht td0? Nicht für ein td-Element, sondern für dessen Textinhalt (transformiert in Großbuchstaben). Die Benennungen td0, td1 und td2 sind also ebenfalls schlecht.

Und warum nicht sowas wie (ungetestet)

    for ( let row of rows ) {

        const cellContents = row.cells.map(x => x.textContent.toUpperCase());
        ⋮
    }

Statt td0, td1 und td2 dann cellContents[0], cellContents[1] bzw. cellContents[2].

---

        tr[i].style.display =
            td0.indexOf( filter ) > -1 ||
            td1.indexOf( filter ) > -1 ||
            td2.indexOf( filter ) > -1
            ? "" : "none";

Nein!! Man sollte i.d.R. niemals mit JavaScript Werte von CSS-Eigenschaften setzen, sondern Änderungen im DOM machen. Zum Verstecken bietet HTML das hidden-Attribut:

        tr[i].hidden = 
            td0.indexOf( filter ) > -1 ||
            td1.indexOf( filter ) > -1 ||
            td2.indexOf( filter ) > -1
            ? false : true;

Wobei man gleich den Booleschen Wert zuweisen kann:

        tr[i].hidden = !(
            td0.indexOf( filter ) > -1 ||
            td1.indexOf( filter ) > -1 ||
            td2.indexOf( filter ) > -1
        );

Mit de Morgan kriegt man auch die Negation weg:

        tr[i].hidden = 
            td0.indexOf( filter ) == -1 &&
            td1.indexOf( filter ) == -1 &&
            td2.indexOf( filter ) == -1;

(Ebenfalls ungetestet. Zu tr[i] und td0, td1, td2 siehe oben.)

🖖 Живіть довго і процвітайте

[borisbaer]

Hallo Gunnar, vielen Dank für die Hinweise! Ich überspringe jetzt mal alles, was mir ersichtlich ist, und gehe mal auf die noch unklaren Passagen ein:

Und warum nicht sowas wie (ungetestet)

    for ( let row of rows ) {

        const cellContents = row.cells.map(x => x.textContent.toUpperCase());
        ⋮
    }

Statt td0, td1 und td2 dann cellContents[0], cellContents[1] bzw. cellContents[2].

Hier kommt es zu folgender Fehlermeldung:
Uncaught TypeError: row.cells.map is not a function
Hat scheinbar ein Problem mit dem row.cells-Array?

Mit de Morgan kriegt man auch die Negation weg:

        tr[i].hidden = 
            td0.indexOf( filter ) = -1 &&
            td1.indexOf( filter ) = -1 &&
            td2.indexOf( filter ) = -1;

Was passiert hier? Warum wird das Vergleichszeichen geändert und das Oder- zu einem Und-Zeichen? Zeigt jedenfalls auch eine Fehlermeldung an: Parsing error: Assigning to rvalue

Grüße
Boris

[Gunnar Bittersmann]

@@borisbaer

Hier kommt es zu folgender Fehlermeldung:
Uncaught TypeError: row.cells.map is not a function
Hat scheinbar ein Problem mit dem row.cells-Array?

Hm, ich sagte ja: ungetestet. Und „sowas wie“. Irgendwie sollte da was mit map zu machen sein. Wie nun genau, müsste ich mir auch erst ansehen.

Mit de Morgan kriegt man auch die Negation weg:

        tr[i].hidden = 
            td0.indexOf( filter ) = -1 &&
            td1.indexOf( filter ) = -1 &&
            td2.indexOf( filter ) = -1;

Was passiert hier? Warum wird das Vergleichszeichen geändert?

Das Stichwort „de Morgan“ hatte ich doch genannt. Danach in der Wikipedia gesucht: De-morgansche Gesetze. (Seltsame Schreibweise mit dem kleinen m.)

In JavaScript-Syntax: !(a || b || c) ist dasselbe wie !a && !b && !c.

Natürlich muss man die Vergleichszeichen richtig ändern …

Zeigt jedenfalls auch eine Fehlermeldung an: Parsing error: Assigning to rvalue

… ==, nicht =. Wie peinlich. Ich berichtige das gleich noch im anderen Posting.

🖖 Живіть довго і процвітайте

[Rolf B]

Hallo Gunnar,

ich finde, dass hidden gut gemeint ist, aber schlecht gemacht. Im Browser ist es realisiert als

[hidden] {
   display: none;
}

und das führt dazu, dass hier

<style>
   nav {
      display: flex;
   }
</style>
...
<nav hidden>...</nav>

das nav Element trotz des hidden-Attributs sichtbar ist (Chrome + Firefox)

Man muss nachsteuern und die browser-interne Regel nochmal im eigenen CSS nachlegen, dann ist es besser. Damit sie sicher gewinnt, sollte es so ziemlich die letzte CSS Regel sein, die der Browser antrifft. Oder man löst das Problem nuklear.

Rolf

[Gunnar Bittersmann]

@@Rolf B

Im Browser ist es realisiert als

[hidden] {
   display: none;
}

Ja, [hidden] { display: none !important } ist sicher sinnvoller.

Es schadet nicht, ebendas in sein Stylesheet zu schreiben.

🖖 Живіть довго і процвітайте

[Rolf B]

Hallo Gunnar,

row.cells.map(x => x.textContent.toUpperCase())

row.cells ist eine HTMLCollection und kein Array. Deswegen gibt's map da nicht.

Array.prototype.map.call(row.cells, x => x.textContent.toUpperCase())

würde funktionieren. Aber ist das lesbar??? Man muss ja ohnehin auf die ersten drei Spalten eingrenzen, und dann könnte man es so machen:

if (Array.from(r.cells)
         .slice(0,3)
         .some(cell => cell.textContent.toUpperCase().indexOf(filter) >= 0)) {
   // treffer
}

_Rolf_

-- 
sumpsi - posui - obstruxi

[borisbaer]

Hallo Rolf,
ich habe es jetzt so umgesetzt, wie du vorgeschlagen hast! Danke!

function filterTable( event ) {

    const filter = event.target.value.toUpperCase();
    const table = event.target.closest( 'table' );
    const rows = table.querySelector( 'tbody' ).rows;

    for ( let row of rows ) {

        row.style.display = Array.from( row.cells )
            .slice( 0, 5 )
            .some( cell => cell.textContent.toUpperCase().indexOf( filter ) >= 0 )
            ? '' : 'none';

    }

}

Ansonsten ist aber auch gut zu wissen, wie man eine HTMLCollection in ein Array umwandeln kann. 🙂 Ich habe mich jetzt doch hier für style.display entschieden, weil ich möchte, dass manche Zeilen auch bei der Suche versteckt bleiben. Die würden sonst ebenfalls angezeigt werden. Ich habe für die Zeilen, nach denen man nicht suchen kann [hidden] { display: none !important; } definiert und für die Suche dann halt ohne das !important.

Grüße
Boris

[encoder]

Eine "für alle" Schleife ist eleganter, kürzer und verständlicher als eine Schleife mit Zähler. Da bin ich schon auch dabei.
Diese Weise um auf den zweiten Zähler zu verzichten macht es aus meiner Sicht allerdings nicht mehr kürzer / eleganter / verständlicher.
Man denke nicht nur an die vielleicht momentan empfundene Eleganz, sondern auch daran dass man irgendwann den Code - meistens unter Zeitdruck - schnell wieder verstehen mus 😉

[encoder]

Du könntest ohne Variablen auskommen indem du tr[i].cells[0].textContent.toUpperCase().indexOf(filter) direkt in das if übernimmst.

Ebenfalls denkbar wäre eine Schleife cells[j] innerhalb der Schleife über i.

for (j = 0; j <=2; j++) {
  if (tr[i].cells[j]...indexOf(filter) > -1) {
    found = 1;
    break;
  }
}
style = found ? ... : ... ;

Was davon schöner ist dürfte Geschmackssache sein. Bei nur drei zu durchsuchenden Spalten macht die zweite Schleife das Ganze eher unübersichtlicher als ausdrücklich jeden Test auszuprogrammieren.

[Gunnar Bittersmann]

@@encoder

Ebenfalls denkbar wäre eine Schleife cells[j] innerhalb der Schleife über i.

for (j = 0; j <=2; j++) {

Noch so einer mit ’ner Uralt-Schleife.

🖖 Живіть довго і процвітайте

[borisbaer]

Hallo encoder,
danke für den Vorschlag! Was die Anzahl der Spalten angeht, möchte ich flexibel sein, weil sich diese je nach Tabelle auch verändern können. Ich muss wohl noch einen Parameter in die Funktion einbauen, der die Menge der zu durchsuchenden Spalten übergibt.

Grüße
Boris