Który z tych dwóch kodów jest dla was bardziej czytelny?

1. Wersja pierwsza, znalezienie pierwszego wystąpienia i przyrównanie go do 0

   private function resetsFlags(): bool
   {
       return \strPos($this->flagString, '^') === 0;
   }
   

2. Wersja druga, sprawdzenie pierwszego znaku pod warunkiem że nie jest pusty

   private function resetsFlags(): bool
   {
       if ($this->flagString === '') {
           return false;
       }
       return $this->flagString[0] === '^';
   }
   

PS: I tak, wiem że istnieje str_starts_with(), wiem że moge napisać swoją wersję startsWith($haystack,$needle), dziękuję.

Żadnych cyrków w stylu ($this->flagString[0] ?? '') === '^' proszę nie proponwać.

nie znam się na phpie, ale może zadziała:

private function resetsFlags(): bool
{
    return $this->flagString != ''
        && $this->flagString[0] === '^';
}

druga, bo fail fast + nie znając PHP wiedziałem co robi, a \ przed strPos zrobił mi wtfa + wypadałoby abym wiedział co dokładnie robi strPos i jak obsługuje błędy

private function resetsFlags(): bool
{
    if ($this->flagString === '')
        return false;

    return $this->flagString[0] === '^';
}

Wibowit napisał(a):

nie znam się na phpie, ale może zadziała:

private function resetsFlags(): bool
{
    return $this->flagString != ''
        && $this->flagString[0] === '^';
}

To prawda, ale moim zdaniem to jest jeszcze mniej czytelne.

1a2b3c4d5e napisał(a):

druga, bo fail fast + nie znając PHP wiedziałem co robi, a \ przed strPos zrobił mi wtfa

Jak się woła normalnie funkcje w PHP (strPos()) to jeden call w C najpierw szuka tej funkcji w aktualnym namespace'ie, a potem dopiero w globalnym. Użycie \strPos() pozwala uniknąć tego dodatkowego calla, można zyskać mikrosekundę w benchmarku. Działa też jawny import use strPos; na górze pliku. W PHP8 to poprawili że dla niektórych funkcji już tak się nie dzieje, z tego co wiem.

1a2b3c4d5e napisał(a):

wypadałoby abym wiedział co dokładnie robi strPos i jak obsługuje błędy

Zwraca położenie w bajtach pierwszego wystąpienia infixu, lub (żyg) false jeśli nie ma takiego.

żadna, funkcja która zwraca bool'a i nazywa się resetsFlags to jest jakis WTF

@Wibowit:

if (something.CanResetsFlags())
{
}

albo jeżeli to prywatna metoda

if (CanResetsFlags())
{
}

z uwagi na to że Can sugeruje tutaj brak side-effectu, czyli dokładnie tak jak w oryginalnym kodzie

private function resetsFlags(): bool
{
    if ($this->flagString === '')
        return false;

    return $this->flagString[0] === '^';
}

gdzie ResetsFlags moim zdaniem jest a) dziwne (to s) oraz b) spodziewałbym się side-effectu

Dla mnie wszystkie wersje są czytelne. To są przecież tylko 2 linijki kodu!

Owszem wersja 1 wymaga "wiedzy" o tym co zwraca strPos() jeśli nie znajdzie znaku a w drugiej nawet osioł się domyśli o co chodzi.
Osobiście bym zapisał w wersji 2 a właściwie jej modyfikacji czyli bez wąsów za if'em. I oczywiście zmieniłbym samą nazwę funkcji bo ta swoją nazwą myli sugerując, że coś "resetuje" (tak jak napisał @kzkzg) a tak nie jest.

katakrowa napisał(a):

Dla mnie wszystkie wersje są czytelne. To są przecież tylko 2 linijki kodu!

I to znaczy że można olać czytelność?

Rozumiem, że jak Ci spadnie 2zł to nie podnosisz? :> Ja bym podniósł

TomRiddle napisał(a):

I to znaczy że można olać czytelność?

Nie można ale w podanych przykładach obie wersje są czytelne.

Nie chcę nic mówić, ale temat się ciut rozrasta, a my tu dyskutujemy tylko o wyższości jednego podejścia do drugiego :) mimo wszystko to fajnie. Ja tam bym wybrał opcję z fail fast, ale na CR jakbyś mnie ktoś przekonał do pierwszej wersji - to nie dyskutowałbym tak długo :)

Aventus napisał(a):

No cóż, pozostaje nam się nie zgodzić :) Ty masz prawo być przekonany o swojej racji, jak i ja- na bazie doświadczeń i konsensusu. Dodam tylko że przykład z index nie jest odpowiedni, bo jeśli ktoś nie wie co to oznacza to brak mu po prostu wiedzy technicznej. Nijak ma się to do nazewnictwa metod czy zmiennych określających konkretne funkcje (nie w sensie matematycznym)

Patrz, tak to wyłumaczę.

Powiedzmy że masz grę, w której są zwierzęta Animal (wilk, lis i prosiaki, WolfAnimal, FoxAnimal i PigAnimal). Wilk może gryźć i zabić wolf.bite(), wolf.kill(). Lis może tylko ugryść fox.bite().

Chcę teraz dodać funkcję, która mi sprawdzi czy jakieś zwierzę zazwyczaj goni inne zwierzęta (jeśli tak, to np pomaluję go na czerwono). Nazwę tą funkcję animal.chasesAnimals() (mimo że żadne ze zwierząt nie ma funkcji .chase()). Nie dlatego że faktycznie sprawdzam czy ona może wykonać funkcję .chase() (bo wtedy pewnie nazwałbym ją .canChaseAnimals(), tylko dlatego że taki jest requirement biznesowy - pomaluję na czerwono te zwierzęta które gonią inne - .chasesAnimals().

.chasesAnimals() jest w mojej opinii podobne do .resetsFlags(). Według naszej rozmowy nie powinienem nazywać tej funkcji animal.chasesAnimals(), tylko jakoś inaczej, animal.shouldChaseAnimals(), animal.isAnimalCnsideredChased(), etc; z czym się nie zgadzam. W mojej opinii zarówno .chasesAnimals() jak i .resetsFlags() są okej.

Mogę przyznać, że mogliście tego nie widzieć w swoich projektach i możecie nie być zaznajomieni z takim sposobem nazywania funkcji, ale to nie czyni ich zaraz niepoprawnymi albo mniej czytelnymi.

Nazwa zupełnie nie pasuje do tego co robi funkcja i nie broni tego żadna konwencja z s.
A teraz krótkie wyjaśnienie: odpalam IDE i szukam klasy sprawdzającej czy mogę te flagi zresetować i bez wgłębienia w kod poszczególnych funkcji po samej nazwie jej nie znajdę.
Po drugie jak chciałbyś nazwać funkcję, która faktycznie wykonuje reset?

Kod który napisał @Wibowit jest najlepszy.
BTW. Czy to pole może być kiedyś pustym stringiem?

Obie wersje są czytelne, tylko co z tego, skoro robią różne rzeczy. Mi osobiście nie podoba się negacja if'em, więc to co napisał @Wibowit dla mnie jest najprzyjemniejsze/wymagające najmniej wysiłku w interpretacji.

TomRiddle napisał(a):

I tak, wiem że istnieje str_starts_with()

Co jest złego w tej funkcji?

@TomRiddle: odpisuję teraz bo- nie wiem czy to była jakaś specjalna zagrywka z Twojej strony- wyciągnąłeś mój komentarz, zacytowałeś go tak jakby to był post, i nie otagowałeś mnie. W rezultacie nie wiedziałem że w ogóle do mnie cokolwiek odpisałeś bezpośrednio do mnie. Komentarz ten jest już nieaktualny, bo później rozmowa toczyła się dalej i co nieco się wyjaśniło. A to wszystko podsumowałeś stwierdzeniem:

Mogę przyznać, że mogliście tego nie widzieć w swoich projektach i możecie nie być zaznajomieni z takim sposobem nazywania funkcji, ale to nie czyni ich zaraz niepoprawnymi albo mniej czytelnymi.

"Mogę przyznać że (...)" czyli innymi słowy "Przyjmę swoją (niekoniecznie zgodną z faktycznym stanem rzeczy) wersję wydarzeń". Oj nie ładnie. Żeby sprawiedliwości stało się za dość, tutaj jest reszta komentarzy:

Twoje wyjaśnienie:

Najdokładniej mówiąc, posługując się wiedzą domenową to ta funkcja sprawdza, czy dany ciąg formatujący "resetuje flagi we wzorcu". Mówimy że ciąg resetuje flagi jeśli zaczyna się od ^. Odpowiednikiem byłoby np new SqlQuery("SELECT * FROM table").addsRecords(); // false, new Query("INSERT INTO table;").addsRecords(); // true. Można popatrzeć na query SQL'owe i stwierdzić czy ono dodaje rekordy czy nie. INSERT dodaje, SELECT nie dodaje. Wtedy funkcja to byłoby bool addsRecord() { return this.query.startsWith("INSERT"); }

Moja odpowiedź:

Ok, chyba już rozumiem o co Ci chodzi. Mam obiekt który reprezentuje (to bardzo ważne) jakiś ciąg, i metodę która determinuje czy ten ciąg zawiera znak, który w szerszym kontekście sprawi że flagi powinny zostać zresetowane. Dobrze rozumiem? A jeśli dobrze rozumiem, to nazwałbym to po prostu ContainsResetFlagsSymbol dla pełnej jasności. I nie, nie jest to sprzeczne z Contains na generycznej kolekcji, bo w tym drugim przypadku przekazujesz jako parametr co konkretnie chcesz znaleźć.