funkcja modyfikująca DOM: argument lokalny czy globalny

Załóżmy, że mam prostką apkę mobilną, na której coś pokazuję. Realizuje to funkcja, którą mogę zdefiniować na 2 sposoby:

1. funkcja działa na elemencie DOM, który jej przekazuję
2. funkcja działa na elemencie DOM, który każdorazowo ona sobie łapie

Czyli może być tak:

const myDIV = document.getElementById("elementId");
function updateData(div, message) {
  div.textContent = message;
  div.stylebackgroundcolor = "red";
}

Albo tak:

function updateData(divId, message) {
  const myDIV = document.getElementById(divId);
  myDIV.textContent = message;
  myDIV.stylebackgroundcolor = "red";
}

Pytanie który sposób lepszy? Osobiście wydaje mi się, że II. Bo trudniej tak będzie zmienić to, co siedzi w globalnym myDIV.

Druga opcja jest o tyle bezpieczniejsza, że pobierasz sobie referencję do elementu, na którym będziesz działać bezpośrednio przed operacją. W sensie - odpalasz funkcję, ona sobie pobiera element, a zaraz potem go zmienia - więc są małe szanse, że coś się zmieni i ten element będzie jakoś nieprawidłowy. Z kolei przy pierwszej opcji moze się stać tak, że do zmiennej myDIV dawno temu zostało coś podstawione i teraz jest to już błędne.

Poza tym druga sprawa (to bardziej już dyskusja w stylu clean code) - po to masz funkcję, żeby ona coś zrobiła. Jeśli ma zmienić wartość jakiegoś elementu, to wszystko co się z tą zmianą wiąże powinno być w tej funkcji, a nie że sobie najpierw pobierasz jakieś uchwyty czy referencje do obiektów/elementów, a dopiero potem odpalasz funkcję. Jeśli pobranie referencji do elementu DOM jest potrzebne tej funkcji do działania, to powinno się to zawierać w jej ciele. Przekazujesz jej co i z czym ma zrobić, cała implementacja powinna być ukryta w jej środku.

Jeszcze jeden argument - oszczędność kodu: w pierwszej wersji za każdym razem musisz wpisać dwie linie/dwa polecenia: pobranie elementu oraz wywołanie funkcji. W drugim wariancie każdorazowo wywołanie jest tylko jedną linią.

wersja pierwsza lepsza, bardziej elastyczna, bo dostajesz jakiś obiekt (koncepcyjnie element DOM) i go zmieniasz i tyle.

Wersja druga jest nieelastyczna - zakładasz, że

• jesteś w środowisku przegladarkowym i masz do dyspozycji obiekt document (w testach odpalanych w Node nie będziesz mógł zrobić document.getElementById, chyba że zamockujesz np. za pomocą biblioteki jsdom. Pewnie, w niektórych miejscach trzeba będzie odwołać się do dokumentu, ale w tej funkcji jest to na siłę. Jakby za dużo odpowiedzialności wrzuconej do jednej funkcji. Pamiętaj, że document to zmienna globalna, która trzyma jakieś globalne drzewko elementów. Czyli twoja funkcja zamiast działać lokalnie, działa na globalnym stanie
• zakładasz, że w rzeczonym w dokumencie będzie element o danym id, co już jest niewygodne po prostu. Nie zawsze się wybiera element za pomocą id, często wygodniej jest przekazać bezpośrednio referencję do elementu
• Po trzecie ciagle pobierasz dany element z DOM co może być mało wydajne

Czyli ogółem: wersja pierwsza to lepszy design, wersja druga choć różni się jedynie linijką, generuje dużo problemów

LukeJL napisał(a):

• Po trzecie ciagle pobierasz dany element z DOM co może być mało wydajne

O to mi chodziło.

W przypadku zmiennych globalnych można zrobić takie zabezpieczenie na wypadek braku elementu w DOMie.

const myDIV = 0;
function captureDiv(elementId) {
  if (document.getElementById(elementId)) {
    myDIV = document.getElementById(elementId);
 }
}

Lepiej mieć błąd / wyjątek i żeby cała apka się wysypała, wtedy widać, że się zrobiło błąd

No to zależy od tego, co chcemy zrobić. Jak w przykładzie z tego wątku - grzebiemy na treści i kolorze jakichś DIV'ów. I takim DIV'em może być cokolwiek - jakiś główny kontener, bez którego nic nie będzie działać, ale równie dobrze jakiś ozdobnik na dole ekranu pokazujący emotke albo obrazek ze śpiącym kotkiem. O ile w pierwszym przypadku dobrze jest zatrzymać aplikacje, ale bez kotka spokojnie da się żyć i pracować :p

Także standardowa odpowiedź numer 27 - "to zależy". W każdym razie - nie popieram rzucania wyjątków i ubijania apki za każdym razem, nawet jeśli błąd jest bardzo drobny.

Ale brak elementu może świadczyć o jakiejś pomyłce programisty albo złych założeniach, więc dobrze, żeby programista to widział od razu i żeby błąd odstraszył go od wrzucenia tego na produkcję. A ukrywając błędy potem takie błędy mogą trafić łatwiej na produkcję i użytkownikowi końcowemu coś się źle wyświetli.

np. załóżmy, że mamy taki kod wrzucony kilka postów wyżej przez @rafal95p

const myDIV = 0;
function captureDiv(elementId) {
  if (document.getElementById(elementId)) {
    myDIV = document.getElementById(elementId);
 }
}

ten kod jest ciekawy, bo zawiera dwa błędy. Jeden jawny, który się od razu wywali i drugi błąd, który przejdzie niezauważony.

Pierwszy błąd czyli błędne użycie const zamiast let - i z tego powodu wywali się od razu, jak if będzie prawdziwy (TypeError: Assignment to constant variable.). I jest to w zasadzie malutki błąd, bo od razu przeglądarka go pokaże i po prostu zamieni się const na let i już. Więc zakładam, że ten kod nie trafi na produkcję w tej postaci.

Jednak jest drugi błąd, gdzie myDIV jest inicjalizowane do 0 (dlaczego do zero, zamiast do null czy undefined?). to jest błąd założeń, który z powodu słabego typowania JS nawet nie zostanie wykryty (w sensie to nie ma sensu, żeby inicjalizować myDIV do zero i gdyby JS był silnie typowany, to by to wykrył). I taki błąd łatwiej może trafić na produkcję i później generować dziwne bugi. np. załóżmy hipotetyczną sytuację, że mamy taki kod:

const myDIV = 0;
//...
myDIV.innerText = "hello";

I taki kod przejdzie cicho, ale potem się zdziwimy czemu tekst nie zmienił się na hello i dopiero debugowanie byłoby potrzebne.

Jeśli funkcję możesz uznać za prywatną, wówczas odpowiedź na Twoje pytanie nie ma żadnego znaczenia.

Jeśli funkcje są publiczne to pomyśl przez chwilę na ile taka funkcja jest trwała?

To co cechuje obie funkcje to fakt, że obie mutują i próbują ukryć szczegóły tej mutacji, ale jeśli z czasem pole z komunikatem będzie obejmować więcej zmian (np. ustawienie ikonki w innym, ale powiązanym elemencie) to obie funkcje będą wymagały modyfikacji argumentów, wyjdzie na to, że to co próbujesz teraz ukryć (szczegóły modyfikacji) wylezie na sam wierzch. Uzyskasz efekt końcowy odwrotny od zamierzanego.

W kwestii wydajności jedna rzecz jest istotna: czy łapiemy jeden element DOMu (Id), czy kilkanaście/kilkadziesiąt (class o określonej nazwie).