JavaScript Optimierung des Scripts

[felsi]

Hallo,

ich bin absoluter Javascript-Neuling und habe mir unter Schweiß und Tränen folgendes Script zusammengeschustert. Ziel ist es, eine CSS Klasse ein/ausblenden zu können sowie einer anderen das text-decoration: line-through zu nehmen sowie wieder hinzuzufügen. Der Schwierigste Teil für mich dabei war, den Status in eine localstorage zu speichern.

Zu meinem eigenen Erstaunen funktioniert mein Script, allerdings ist es programmiertechnisch eine Katastrophe (denke ich):

//Toggle price
jQuery( document ).ready(function( $ ) {
//if localstorage item is not empty set '.special-price' to 'visibility: hidden'  
if (window.localStorage.getItem("special-price") != null) {
    var sp = window.localStorage.getItem("special-price");
    if (sp === "true") {
      $(".special-price").css("visibility", "hidden");
    }
  }
   
//AND if localstorage item is not empty set '.old-price' to 'text-decoration: none'  
if (window.localStorage.getItem("price") != null) {
    var op = window.localStorage.getItem("price");
    if (op === "true") {
      $(".old-price .price").css("text-decoration", "none");
    }
}
//click .btn-hide to hide .special-price and save in localstorage  
    $(".btn-hide").click(function() {
    var v = $(".special-price").is(":visible");
    $(".special-price").css("visibility", "hidden");
    window.localStorage.setItem("special-price", v);
  });
//click .btn-hide to hide line-through of .old-price and save in localstorage  
    $(".btn-hide").click(function() {
    var n = $(".old-price .price").is(":visible");
    $(".old-price .price").css("text-decoration", "none");
    window.localStorage.setItem("price", n);
  });
//click .btn-show to show .special-price and clear localstorage
    $(".btn-show").click(function() {
    $(".special-price").css("visibility", "visible");
    localStorage.clear();
  });
//click .btn-show to add text-decoration line-through and clear localstorage    
    $(".btn-show").click(function() {
    $(".old-price .price").css("text-decoration", "line-through");
    localStorage.clear();
  });
});

Nun wollte ich fragen, ob mir eventuell jemand den Code etwas aufräumen könnte bzw. mir Tipps geben könnte, wie ich es optimieren kann.

Außerdem stört mich noch, dass ich zwei Buttons benötige (einen zum Ausblenden und einen zum Einblenden). Gibt es eine Möglichkeit, beides in einem Button zu integrieren?

Vielen Dank bereits im Voraus.

 

[benneq]

Schreib dir Helfer Funktionen für Sachen, die du öfter benötigst.
Und benutz Variablen für Dinge, die du mehrfach benötigst.

 

[DualityMind]

Wie ein Einschalt/Ausschalter mit Werten 1 und 0 um einen einzigen Button zu benutzen.

 

[cRz7]

Da du ja auch jQuery benutzt kannst du auch eine CSS Klasse erstellen und sie mit toggle() hinzufügen bzw wegnehmen.

 

[felsi]

Da du ja auch jQuery benutzt kannst du auch eine CSS Klasse erstellen und sie mit toggle() hinzufügen bzw wegnehmen.

Ja, das hatte ich zuerst, allerdings konnte ich dieses Script nicht um localstorage ergänzen. Wie gesagt, ich bin absoluter Anfänger und war froh, dass ich das obere Script irgendwie hinbekommen habe.

 

[psYcho-edgE]

Schreib dir Helfer Funktionen für Sachen, die du öfter benötigst.
Und benutz Variablen für Dinge, die du mehrfach benötigst.

Genau das. Du kannst anstatt vier anonymer Funktionen, zwei Funktionen mit Parametern nutzen.

Falls du das nicht willst empfehle ich dir zumindest im if-Block die Variablen mit "let" anstatt "var" zu initialisieren, da dies weniger Speicher benötigt. (Variablen mit let sind nur im jeweiligen Block gültig und werden danach wieder aus dem Speicher geschmissen, je nachdem, wie viele Elemente deine Variable durch den Selektor enthält, kann das durchaus ein bisschen Performance bringen).

Da du ja auch jQuery benutzt kannst du auch eine CSS Klasse erstellen und sie mit toggle() hinzufügen bzw wegnehmen.

Außerdem kannst du auch das noch machen.


Per se würde ich aber sogar jQuery rausnehmen, da dein Anwendungsfall sich ohne die jQuery-Bibliothek auch recht einfach umsetzen lässt und jQuery einen gewissen Overhead mit sich schleppt den du einsparen könntest. Falls du jQuery nicht auch noch für andere Dinge dringend benötigst.

Mich würde außerdem interessieren, was du mit dem local storage anfängst? Du hältst du dort die ganzen jQuery-Objekte drin. Das ist ganz schön viel Speicher für Umme, falls du die Objekte nicht an anderer Stelle noch benötigst. Falls es dir nur um irgendwelche IDs oder HTML-Eigenschaften der jeweiligen Objekte geht würde ich einfach jeweils ein kleines neues Objekt mit den jeweilig benötigten Eigenschaften im storage halten.

Liebe Grüße und viel Erfolg. Falls du genaue Infos brauchst, kannst du gern ne PM schreiben

 

[felsi]

Falls du das nicht willst

Das ist weniger eine Sache des Willens und mehr des Könnens. Deine Erläuterung klingt logisch, aber ich habe absolut keine Ahnung wo ich ansetzen muss. Ich habe schon mehrmals versucht Javascript von Grund auf zu lernen, aber ich glaube, ich bin einfach nicht zum Programmieren geeignet - leider, würde es sehr gerne können

Falls du jQuery nicht auch noch für andere Dinge dringend benötigst.

Die Seite bringt die jQuery Bibliothek sowieso schon mit und nutzt sie auch für andere Dinge. Falls du allerdings ein wenig Zeit hast, und mir das Script entsprechend umstellen könntest (oder mir dabei helfen), wäre ich natürlich super dankbar

 

[psYcho-edgE]

Ich meld mich mal nach Feierabend (halb/um sechs), dann können wir schauen was sich machen lässt. Ich bin aber auch nicht der absolute JS-Vollprofi, hab nur etwas länger schon damit zu tun

 

[rob-]

Da du ja auch jQuery benutzt kannst du auch eine CSS Klasse erstellen und sie mit toggle() hinzufügen bzw wegnehmen.

Richtig. CSS gehört nich in JavaScript Code. Das Maximum der Dinge sollte das setzen / togglen von Klassen sein.
Außerdem hast du viele Stellen mit gleichem Code, das kannst in Funktionen auslagern und den CSS selector übergeben.

 

[mastaqz]

Paar Ideen:

- du benutzt ein paar Variablen, die du nur einmalig benutzt und dann nicht mehr; kannst du prinzipiell löschen; oder zumindest Namen geben, die erklären, was die Variable hält
- du änderst über .css() das Styling, das wäre als Klasse sinnvoller; eine Klasse in CSS erstellen und über JS nur togglen; dann ist es auch einfacher, sollten zu dieser Klasse noch zusätzliche Inhalte kommen
- du hast für jeden Button zwei Listener gestellt, das kann jeweils in einen hinein
- deine Kommentare sind sehr nichtssagend, fassen bei dir lediglich zusammen, was du dann im Code machst => "Code Tells You How, Comments Tell You Why"