Czy piszecie słowo this w metodach?

Aventus napisał(a):

EDIT: Dodam jeszcze że raz pracowałem w projekcie gdzie dla pól ani nie używano this, ani nie używano podkreślnika. Z dwojga "złego" zdecydowanie już wolał bym używać this niż nie używać nic, bo wtedy kod jest zdecydowanie mniej czytelny:

No właśnie, to jest chyba sedno problemu. Jeśli nie użyjesz this, to aby nie zwariować musisz jakoś te pola wyróżniać, np. tym okropnym podkreślnikiem na początku.

Grzyboo napisał(a):

Nie macie kolorowania składni w VisualStudio czy w czym się tam aktualnie programuje C#? :P

Mamy, ale nie mamy jeszcze tego poziomu paranoi, aby code review robić każdorazowo w IDE.

Saalin napisał(a):

Nie piszę this jeśli nie muszę, a muszę rzadko. Poza tym explicit nie zawsze jest lepsze niż implicit, jak się ludzie przyzwyczaja do pisania niepotrzebnego kodu to im się później wydaje że inaczej się nie da, nie będzie działać albo będzie działać inaczej - np. wywołanie bezparametrowego konstruktora klasy bazowej w C# może być implicit lub explicit, ale przyzwyczajenie się do pisania tego może budować mylne wrażenie, że bez tego base() konstruktor się nie wywoła. Podobny syf jak pisanie pustych konstruktorów, bo to też przecież jest explicit, zamiast polegania na konstruktorze domyślnym.

Trochę nie wiem jak można porównywać dodawanie zbędnych wywołań albo elementów klasy ze sposobem zapisu dostępu do elementów klasy. Pomiędzy postawieniem średnika na końcu instrukcji SQL, a dopisaniem AND 1=1 do WHERE też postawiłbyś znak równości?

@somekind:

No właśnie, to jest chyba sedno problemu. Jeśli nie użyjesz this, to aby nie zwariować musisz jakoś te pola wyróżniać, np. tym okropnym podkreślnikiem na początku.

Dokładnie, jeśli nie użyjesz podkreślnika to aby nie zwariować musisz jakoś te pola wyróżniać, np. tym okropnym this :p

Kopiuj

void DoSomething()
{
  // ...
  finalResult = currentResult + someValueToAdd; // Które z tych to pola, a które to zmienne lokalne?
}

Zakomentowałeś ciało funkcji i pytasz nas o jej elementy? :D

Odpowiedź jest prosta, jeśli któraś z tych zmiennych jest zadeklarowana w ciele funkcji DoSomething, to to zmienna lokalna.

Klasy i funkcje w każdym projekcie powinny być małe i krótkie. Klasa lub funkcja w której nie jest jasne czy jest polem czy zmienną lokalną jest zbyt duża.

Czy problemem jest brak this przy użyciu pół, i to sprawia że klasa jest nie czytelna? Nie, to klasa i jej funkcje są zbyt duże.

W poprawnie napisanej, czystej klasie jest kilka-kilkanaście method z których każda ma 1-5, może -1-10 linijek. Programista powinien jednym rzutem oka dostrzec deklaracje pół oraz deklaracje zmiennych lokalnych, i na podstawie ich scope'u wiedzieć czym są.

Dodatkowo nazwy pól i zmiennych lokalnych (jeśli ktoś nazwał je dobrze) też często pomagają.

Zakomentowałeś ciało funkcji i pytasz nas o jej elementy? :D

Tak, taki był sens tego eksperymentu myślowego.

Odpowiedź jest prosta, jeśli któraś z tych zmiennych jest zadeklarowana w ciele funkcji DoSomething, to to zmienna lokalna.

Thanks captain obvious :D Ale tak serio to właśnie potwierdzasz to że brak widocznego na pierwszy rzut oka zakresu zmiennych sprawia że musisz patrzyć na szerszy kontekst (metody lub klasy), czyli wymusza to większy wysiłek mentalny i rozpraszanie się. A więc kod jest zwyczajnie mniej czytelny.

Klasy i funkcje w każdym projekcie powinny być małe i krótkie. Klasa lub funkcja w której nie jest jasne czy jest polem czy zmienną lokalną jest zbyt duża.

Zgadzam się co do argumentu o długości klas i metod, natomiast nijak ma się to do czytelności kodu pod względem zakresu zmiennych. Patrz to co napisałem wyżej.

Czy problemem jest brak this przy użyciu pół, i to sprawia że klasa jest nie czytelna? Nie, to klasa i jej funkcje są zbyt duże.

Jeśli musisz utrzymywać w umyśle szerszy kontekst analizując dany kawałek kodu (aby wiedzieć czy dana zmienna to zmienna lokalna czy pole klasy) to z definicji taki kod jest mniej czytelny.

W poprawnie napisanej, czystej klasie jest kilka-kilkanaście method z których każda ma 1-5, może -1-10 linijek. Programista powinien jednym rzutem oka dostrzec deklaracje pół oraz deklaracje zmiennych lokalnych, i na podstawie ich scope'u wiedzieć czym są.

Znów, zgadzam się co do długości metod i klas, ale nie co do tego że rozwiązuje to problem z zakresem zmiennych.

Dodatkowo nazwy pól i zmiennych lokalnych (jeśli ktoś nazwał je dobrze) też często pomagają.

Jestem ciekaw w jaki sposób to może pomagać. Będziesz w nazwie każdego pola dawał field czy bawił się w jakieś inne kwiatki? Przykład: currencyConverter. Jak sprawisz aby nazwa odzwierciedlała że jest zmienną lokalną, a jak że jest polem klasy?

Osobiście nigdy nie pisałem, ale niektóre argumenty w poprzednich postach sprawiły, że zacznę.

W Eliksirze (i Erlangu) jest pewna różnica między:

foo() # (1) local call

Oraz

__MODULE__.foo() # (2) remote call
# W Erlangu to będzie ?MODULE:foo()

Różnica jest taka, że (1) (local call) zawsze wywoła metodę, którą widzimy obecnie w kodzie, a (2) (remote call) wywoła metodę w module, który jest "najnowszy". Oczywiście prywatne funkcje można wywołać tylko jako local call.

By zobaczyć różnicę można napisać sobie taki kod:

defmodule Foo do
  def remote_call do
    IO.puts("Remote call v1")
    Process.sleep(500)
    __MODULE__.remote_call()
  end

  def local_call do
    IO.puts("Local call v1")
    Process.sleep(500)
    local_call()
  end
end

spawn_link(Foo, :remote_call, [])
spawn_link(Foo, :local_call, [])

Process.sleep(2_000)

IO.puts("Update `Foo` module")
defmodule Foo do
  def remote_call do
    IO.puts("Remote call v2")
    Process.sleep(500)
    __MODULE__.remote_call()
  end

  def local_call do
    IO.puts("Local call v2")
    Process.sleep(500)
    local_call()
  end
end

Process.sleep(2_000)

Jak odpalimy to jako skrypt to zobaczymy, że po jakimś czasie (2 sekundach) zmieni nam się Remote call v1 na Remote call v2 gdzie Local call v1 pozostanie bez zmian, mimo podmiany kodu modułu. Oba podejścia mają swoje zalety i wady, ale to nie jest temat na to.

somekind napisał(a):

Trochę nie wiem jak można porównywać dodawanie zbędnych wywołań albo elementów klasy ze sposobem zapisu dostępu do elementów klasy. Pomiędzy postawieniem średnika na końcu instrukcji SQL, a dopisaniem AND 1=1 do WHERE też postawiłbyś znak równości?

Średnik w SQL tak jak średnik w JS może zmienić działanie, this w przypadku dostępu do pola nie zmienia nic. Poza tym to kwestia gustu jak tabulatory i spacje - mi bardziej leżą podkreślniki niż this.

@Aventus: no dobra, nie rozumiem tego argumentu o rozróżnianiu. Jak masz konlikt w konstruktorze to używasz this, a w reszcie IDE Ci podpowie, przynajmniej u mnie to działa.

@scibi_92: w jaki sposób IDE Ci podpowie? Zaznaczam raz jeszcze że kolorowanie składni nie wystarczy bo te się może różnić. Tym bardziej jeśli mowa o code review czyjegoś kodu- to najczęściej się robi w przeglądarce, w odpowiednim narzędziu (Azure DevOps, GitHub, GitLab itp.) a tam już nie ma IDE które w jakikolwiek Ci podpowie (to się oczywiście powoli zmienia). Oraz co z projektami gdzie programiści mają wolność w wyborze IDE? W mojej obecnie pracy niektórzy korzystają z VS, a niektórzy z Rider.

Jeśli poprzez "podpowie" masz na myśli że przesuniesz kursor nad zmienną aby zobaczyć czy to pole klasy, to wracamy do mojego argumentu o tym że trzeba bardziej się rozpraszać czytając kod, a więc jest on mniej czytelny.

Nadal więc uważam że z dwojga złego lepiej używać this niż nie używać nic (choć w C# i tak wolę podkreślniki). Dodam jeszcze że doskonale zdaje sobie sprawę że często w grę wchodzą osobiste preferencje. Ja zawsze jestem otwarty na zmiany i próbowanie innego podejścia. Często po wypróbowaniu czegoś zmieniam zdanie (Tak było chociażby z regionami, których kiedyś byłem zwolennikiem. O zgrozo!) ale są takie rzeczy które nawet po dłuższym praktykowaniu nadal do mnie nie przemawiają, a wręcz utrudniają mi życie. Tak było właśnie z nie używaniem niczego specjalnego do pól klas. Przez 2 lata byłem w projekcie gdzie takie podejście właśnie stosowaliśmy bo taka była konwencja zaakceptowana przez "zarząd" techniczny, i moje negatywne nastawienia do takiego podejścia się nie zmieniło.

Piszę w dużej mierze funkcyjnie (tzn. tam gdzie uważam, że to się opłaca), więc zarówno zapis:

cośTam = cośInnego

jest tak samo zły jak ten:

this.cośTam = this.cośInnego

bo oba mają efekty uboczne :]

Jeśli przyjrzymy się takiemu bardziej funkcyjnemu kodowi typu:

val noweCośTam = stareCośTam1 + stareCośTam2

to co za różnica czy to stareCośTamX jest zmienną lokalną czy instancyjną?

W Scali idę nawet dalej, bo można importować ze stałych (czy tam stabilnych) ścieżek (taki tam żargon, możecie się nie przejmować). Np:

val testFixture = TestFixture("abc", 1, shuffle = true)
service.findItem(testFixture.user, testFixture.item1Name) mustBe Seq(testFixture.item1)
service.findItem(testFixture.user, testFixture.item2Name) mustBe Seq(testFixture.item2)

mogę zamienić na:

val testFixture = TestFixture("abc", 1, shuffle = true)
import testFixture._
service.findItem(user, item1Name) mustBe Seq(item1)
service.findItem(user, item2Name) mustBe Seq(item2)

To jest często stosowane podejście w Scali.

Teraz trochę o językach w których nie piszę (Kotlin) lub piszę mało (Rust):

Kotlin udostępnia bajer o nazwie receiver functions czy jakoś tak. Tutaj to w ogóle jest już magicznie. Nie chodzi o implicite this. ani nawet o import cośtam._. Wywołanie receiver function sprawia, że w zasięgu podawanej lambdy pojawiają się nam nowe składowe.
https://kotlinlang.org/docs/lambdas.html#function-literals-with-receiver

class HTML {
    fun body() { ... }
}

fun html(init: HTML.() -> Unit): HTML {
    val html = HTML()  // create the receiver object
    html.init()        // pass the receiver object to the lambda
    return html
}

html {       // lambda with receiver begins here
    body()   // calling a method on the receiver object
}

Rust umożliwia przesłanianie zmiennych lokalnych i przy okazji dowolne zmienianie ich typu, np:

fn main() {
    let a = 5 * 8 + 3; // bez 'mut', a więc niemutowalny integer
    let mut a = a.to_string(); // a tutaj zmienna 'a' zamieniła się w mutowalnego stringa
    a += " apples";
    println!("{}", a);
}

Powyższe wypisuje 43 apples. Jak widać w Ruście nie wystarczy tylko śledzić zagnieżdżenia (żeby szybko wyszukać skąd się wzięła dana zmienna lokalna), bo na tym samym poziomie możemy przesłonić zmienną lokalną.

PS:
A jeśli chodzi o moje doświadczenia z czasów jak jeszcze byłem javowcem to było generalnie tak:

• nie widziałem żadnego projektu, w którym namiętnie pisano by this.
• raz miałem fazę na dopisywanie final przed każdą zmienną lokalną czy parametrem, które miały być z założenia stałymi (ofkoz final w Javie jest płytki, ale nie zmienia to podejścia). Koledzy wybili mi to z głowy twierdząc, że zaśmieca to kod, a niewiele wnosi w czysto imperatywnym języku (to było na długo przed nawet Javą 8).

Aventus napisał(a):

Dokładnie, jeśli nie użyjesz podkreślnika to aby nie zwariować musisz jakoś te pola wyróżniać, np. tym okropnym this :p

Tylko, że this to słowo kluczowe, które niesie ze sobą pewną semantykę (np. że chodzi nam o this, a nie o base).
A rozpoczynanie nazw zmiennych konkretnym znakiem nie niesie semantyki, jest jedynie konwencją, i kojarzy się z takimi arcydziełami jak SQL czy PHP. A stosując duck typing - jeśli coś wygląda słabo, to jest słabe. ;)

Osobiście nie uważam tego za jakiś wielki problem, mogę pisać tak i tak, podobnie jak np. mogę pić herbatę w pokoju pomalowanym na żółto, mimo że sam bym tak nie pomalował.
Osobiście znacznie bardziej razi mnie stawianie spacji między nazwą metody, pozostawianie pustych linii przed klamrą kończącą metodę/klasę albo używanie bezmyślnych komentarzy //arrange //act //assert w kodzie testów.

@somekind: zgadzam się. Jak już kilkakrotnie pisałem to i tak rozbija się o preferencje w tym przypadku. Bo ja np. uważam że semantyka tutaj ma niewiele do rzeczy. Wręcz przeciwnie- uważam że nadmierne wciskanie słów kluczowych w kodzie tam gdzie tego nie trzeba powoduje niepotrzebny hałas w kodzie. Ale znów, to tylko moje zdanie (choć i również większości programistów z którymi miałem do czynienia). Punk widzenia zależy od punktu siedzenia, wiadomo ;)

Choć muszę przyznać że nie wiem co ma duck typing do tego (podkreśników). W ogóle nie widzę związku.

Ale ustalmy jedno. this.foo = foo jest ok jak juz musisz pisac recznie konstruktory bo jezyk tego nie ogarnia za Ciebie. Ale pisanie wszedzie gdzie sie da this - nawet jak nie ma konfliktu nazw - to zupelnie inna para kaloszy.

Aventus napisał(a):

@somekind: zgadzam się. Jak już kilkakrotnie pisałem to i tak rozbija się o preferencje w tym przypadku. Bo ja np. uważam że semantyka tutaj ma niewiele do rzeczy. Wręcz przeciwnie- uważam że nadmierne wciskanie słów kluczowych w kodzie tam gdzie tego nie trzeba powoduje niepotrzebny hałas w kodzie.

No niby nie trzeba, ale z drugiej strony od razu widać, czy odwołujemy się do metody instancyjnej danej klasy, czy bazowej, czy może statycznej. Bez this chwilę zajmuje sprawdzenie tego. A tak można zobaczyć pewne anomalie na pierwszy rzut oka. :)

Zaznaczam raz jeszcze że kolorowanie składni nie wystarczy bo te się może różnić.

Korzystam z IntelliJ i prawie wszyscy z tego korzystają (przynajmniej ci z którymi współpracuje). Dla mnie więc kolorowanie składni wystarcza.

Tym bardziej jeśli mowa o code review czyjegoś kodu- to najczęściej się robi w przeglądarce, w odpowiednim narzędziu (Azure DevOps, GitHub, GitLab itp.)

Dla mnie robienie review jakiejś zmiany która nie jest na kilka lini kodu w Gitlabie to śmiech na sali, puluję brancha i sprawdzam w IntelliJ, więc problem mnie nie dotyczy :P

@scibi_92: no dobra ale to pisz od razu że nie rozumiesz jakiegoś argumentu bo patrzysz przez pryzmat własnego podwórka, a nie ja się rozpisuję bardziej ogólnikowo :p W Twoim przypadku i Twojego zespołu może być tak że faktycznie Wam się to sprawdza, ale w żaden sposób nie jest to uniwersalne.

No w Javie zdecydowana większość programistów kodzi z użyciem IntelliJ, więc pierwszy argument (różne kolory w różnych IDE) zdecydowanie dla tego języka odpada :P

scibi_92 napisał(a):

Dla mnie robienie review jakiejś zmiany która nie jest na kilka lini kodu w Gitlabie to śmiech na sali, puluję brancha i sprawdzam w IntelliJ, więc problem mnie nie dotyczy :P

Śmiechem na sali są PRy tak duże, żeby nie dało się ich zrozumieć bez pobierania kodu.

scibi_92 napisał(a):

No w Javie zdecydowana większość programistów kodzi z użyciem IntelliJ, więc pierwszy argument (różne kolory w różnych IDE) zdecydowanie dla tego języka odpada :P

Że IntelliJ jest upośledzony to wiedziałem, ale aż tak, że nie można zmieniać kolorowania?!

Śmiechem na sali są PRy tak duże, żeby nie dało się ich zrozumieć bez pobierania kodu.

No część osób robi review bez pobierania kodu, ale dla mnie to jest po prostu mało wygodne. 20 sekund na zmiane brancha jest warte tego.

Że IntelliJ jest upośledzony to wiedziałem, ale aż tak, że nie można zmieniać kolorowania?!

Z tego co wiem to można, jak najbardziej - jeszcze jak!

Macie jakieś realne przykłady problemów przed którymi chroni ten nadmiarowy this.? Mi jakoś nigdy do głowy nie przyszła myśl typu: "gdybym miał tutaj this. to wyraźnie szybciej rozwiązałbym problem".

Czytam ten temat i tak jakbym widział dawne kłótnie typu "spacje czy tabulatory" xD

imo this poza konstruktorem to code smell

Zazwyczaj tylko w konstruktorach i setterach - i w idealnym świecie tak być powinno.

Natomiast w świecie rzeczywistym zdarza się używać klas z jakimiś dzikimi polami protected, pochodzących z klasy wyżej - wtedy this przydaje się by rozwiać wątpliwości skąd pochodzi zmienna lub metoda. Kolorowanie składni pomaga, ale jednak taki twardy znacznik jest czytelniejszy jakoś.