Nasza baza kodów jest stara i nowi programiści, tacy jak ja, szybko uczą się robić to tak, jak robi się to dla zachowania jednolitości. Myśląc, że musimy gdzieś zacząć, wziąłem na siebie refaktoryzację klasy posiadacza danych jako takiej:

• Usunąłem metody ustawiające i utworzyłem wszystkie pola final(przyjmuję „ finaljest dobry” aksjomatycznie). Setery były używane tylko w konstruktorze, jak się okazuje, więc nie miało to żadnych skutków ubocznych.
• Wprowadzono klasę Builder

Klasa Builder była konieczna, ponieważ konstruktor (który spowodował przede wszystkim refaktoryzację) obejmuje około 3 wiersze kodu. Ma wiele parametrów.

Na szczęście mój kolega z zespołu pracował nad innym modułem i potrzebował seterów, ponieważ wymagane przez niego wartości stały się dostępne w różnych punktach przepływu. Tak więc kod wyglądał następująco:

public void foo(Bar bar){
    //do stuff
    bar.setA(stuff);
    //do more stuff
    bar.setB(moreStuff);
}

Argumentowałem, że powinien zamiast tego użyć konstruktora, ponieważ pozbycie się seterów pozwala polom pozostać niezmiennymi (wcześniej słyszeli, jak mówiłem o niezmienności), a także dlatego, że konstruktory pozwalają na tworzenie obiektów transakcyjnych. Naszkicowałem następujący pseudokod:

public void foo(Bar bar){
    try{
        bar.setA(a);
        //enter exception-throwing stuff
        bar.setB(b);
    }catch(){}
}

Jeśli ten wyjątek barzostanie uruchomiony , będą miały uszkodzone dane, których można by uniknąć za pomocą konstruktora:

public Bar foo(){
        Builder builder=new Builder();
    try{
        builder.setA(a);
        //dangerous stuff;
        builder.setB(b);
        //more dangerous stuff
        builder.setC(c);
        return builder.build();
    }catch(){}
    return null;
}

Moi koledzy z drużyny odparli, że omawiany wyjątek nigdy nie zostanie uruchomiony, co jest wystarczające dla tego konkretnego obszaru kodu, ale wierzę, że brakuje lasu dla drzewa.

Kompromis polegał na przywróceniu starego rozwiązania, a mianowicie użyciu konstruktora bez parametrów i ustawieniu wszystkiego za pomocą seterów zgodnie z potrzebami. Uzasadnieniem było to, że to rozwiązanie jest zgodne z zasadą KISS, którą kopalnia narusza.

Jestem nowy w tej firmie (mniej niż 6 miesięcy) i jestem w pełni świadomy, że ją straciłem. Mam pytania:

• Czy istnieje inny argument za używaniem Konstruktorów zamiast „starego sposobu”?
• Czy zaproponowana przeze mnie zmiana naprawdę jest tego warta?

ale naprawdę,

• Czy masz jakieś wskazówki, jak lepiej przedstawiać takie argumenty, opowiadając się za próbą czegoś nowego?

Myśląc, że musimy gdzieś zacząć, wziąłem na siebie refaktoryzację klasy posiadacza danych

To właśnie tam poszło nie tak - dokonałeś poważnej zmiany w architekturze bazy kodu, tak że stała się ona zupełnie inna niż oczekiwano. Zaskoczyłeś swoich kolegów. Niespodzianka jest dobra tylko wtedy, gdy obejmuje imprezę, zasada najmniejszej niespodzianki jest złota (nawet jeśli dotyczy imprez, to wiedziałbym, żeby nosić czyste majtki!)

Teraz, jeśli uważasz, że ta zmiana była warta zachodu, znacznie lepiej byłoby naszkicować pseudokod przedstawiający zmianę i przedstawić ją kolegom (lub przynajmniej temu, kto ma rolę najbliższą architektowi) i zobaczyć, co pomyśleli, zanim cokolwiek zrobisz. W tej chwili oszalałeś, pomyślałeś, że cała baza kodów jest twoja do grania według własnego uznania. Gracz w drużynie, nie gracz zespołowy!

Mogę być dla ciebie trochę szorstki, ale byłem w przeciwnym miejscu: sprawdź kod i pomyśl „WTF się tu stało ?!”, tylko po to, by znaleźć nowego faceta (prawie zawsze nowego) zdecydował się zmienić mnóstwo rzeczy opartych na tym, co według niego powinno być „właściwą drogą”. Wkręca się także w historię SCM, co jest kolejnym poważnym problemem.

Na szczęście mój kolega z zespołu pracował nad innym modułem i potrzebował seterów, ponieważ wymagane przez niego wartości stały się dostępne w różnych punktach przepływu.

Jak opisano, nie widzę, co powoduje, że kod wymaga ustawiaczy. Prawdopodobnie nie wiem wystarczająco dużo o przypadku użycia, ale dlaczego nie poczekać, aż wszystkie parametry będą znane, przed utworzeniem wystąpienia obiektu?

Alternatywnie, jeśli sytuacja wymaga setterów: zaoferuj sposób na zamrożenie kodu, aby setery zgłaszały błąd asercji (inaczej błąd programisty), jeśli spróbujesz zmodyfikować pola po przygotowaniu obiektu.

Myślę, że usunięcie seterów i użycie finału to „właściwy” sposób na zrobienie tego, a także wymuszenie niezmienności, ale czy to naprawdę powinieneś spędzać czas?

Ogólne wskazówki

Stary = zły, nowy = dobry, na swój sposób, na swój sposób ... ten rodzaj dyskusji nie jest konstruktywny.

Obecnie sposób użytkowania obiektu jest zgodny z pewną niejawną regułą. Jest to część niepisanej specyfikacji twojego kodu i twój zespół może pomyśleć, że nie ma większego ryzyka dla innych części kodu niewłaściwego użycia. To, co chcesz zrobić, to uwydatnić regułę za pomocą Konstruktora i kontroli czasu kompilacji.

W pewien sposób refaktoryzacja kodu jest powiązana z teorią Broken Window : uważasz, że poprawne jest rozwiązywanie każdego problemu (lub zanotowanie późniejszego spotkania „poprawy jakości”), dzięki czemu kod zawsze wygląda przyjemnie do pracy, jest bezpieczny i czysty. Jednak ty i twój zespół nie macie pojęcia, co jest „wystarczająco dobre”. To, co widzisz jako okazję do przyczynienia się i ulepszenia kodu w dobrej wierze, jest prawdopodobnie postrzegane inaczej niż w istniejącym zespole.

Nawiasem mówiąc, nie należy zakładać, że twój zespół musi cierpieć z powodu bezwładności, złych nawyków, braku wykształcenia ... najgorsze, co możesz zrobić, to stworzyć obraz tego, co jest wszystkim, co jest w stanie je pokazać jak kodować. Na przykład niezmienność nie jest niczym nowym , twoi rówieśnicy prawdopodobnie czytali o tym, ale tylko dlatego, że ten temat staje się popularny na blogach, nie wystarcza, aby przekonać go do poświęcenia czasu na zmianę kodu.

W końcu istnieją nieskończone sposoby na ulepszenie istniejącej bazy kodu, ale kosztuje to czas i pieniądze. Mogę spojrzeć na twój kod, nazwać go „starym” i znaleźć rzeczy do zrobienia. Na przykład: brak formalnej specyfikacji, niewystarczająca liczba zapewnień, może za mało komentarzy lub testów, niewystarczające pokrycie kodu, nieoptymalny algorytm, zbyt duże zużycie pamięci, niestosowanie w każdym przypadku „najlepszego” wzorca projektowego itp. Ad nauseam . Ale dla większości punktów, które chciałbym poruszyć, pomyślałbyś: w porządku, mój kod działa, nie trzeba poświęcać mu więcej czasu.

Być może twój zespół uważa, że ​​to, co sugerujesz, przekroczyło punkt malejących zysków. Nawet jeśli znalazłeś rzeczywistą instancję błędu z powodu pokazanego przykładu (z wyjątkiem), prawdopodobnie po prostu naprawiliby to raz i nie chcieli refaktoryzować kodu.

Pytanie dotyczy tego, jak spędzić czas i energię, biorąc pod uwagę, że są one ograniczone ? W oprogramowaniu wprowadzane są ciągłe zmiany, ale generalnie Twój pomysł zaczyna się od wyniku ujemnego, dopóki nie podasz dobrych argumentów. Nawet jeśli masz rację co do korzyści, sam w sobie nie jest wystarczającym argumentem, ponieważ zakończy się w koszyku „ Miło mieć, pewnego dnia ... ”.

Przedstawiając swoje argumenty, musisz wykonać kilka testów „rozsądku”: próbujesz sprzedać pomysł czy sam? Co jeśli ktoś inny przedstawił to zespołowi? Czy znajdziesz jakieś wady w ich rozumowaniu? Czy próbujesz zastosować ogólną najlepszą praktykę, czy wziąłeś pod uwagę specyfikę swojego kodu?

Następnie upewnij się, że nie będziesz wyglądać tak, jakbyś próbował naprawić przeszłe „błędy” zespołu. Pomaga pokazać, że nie jesteś swoim pomysłem, ale możesz rozsądnie przyjąć informacje zwrotne. Im częściej to robisz, tym więcej sugestii będzie można zastosować.

Jak mogę promować stosowanie wzorca Konstruktora w moim zespole?

Ty nie.

Jeśli twój konstruktor ma 3 linie parametrów, jest zbyt duży i zbyt skomplikowany. Żaden wzór projektowy tego nie naprawi.

Jeśli masz klasę, której dane są dostępne w różnym czasie, powinny to być dwa różne obiekty lub klient powinien agregować komponenty, a następnie skonstruować obiekt. Nie potrzebują do tego konstruktora.

Wydajesz się bardziej skoncentrowany na racji niż na dobrej współpracy z innymi. Co gorsza, zdajesz sobie sprawę, że to, co robisz, jest walką o władzę, a jednak nie zastanawiasz się, jak mogą się czuć ludzie, których doświadczenie i autorytet są dla ciebie wyzwaniem.

Odwróć to.

Skoncentruj się na pracy z innymi. Ułóż swoje myśli w formie sugestii i pomysłów. Poproś JE, aby omówili SWOJE pomysły, zanim zadadzą pytania, które pomogą im pomyśleć o twoim. Unikaj bezpośrednich konfrontacji i bądź na zewnątrz skłonny zaakceptować fakt, że robienie rzeczy po drodze przez grupę jest bardziej produktywne niż toczenie ciężkiej bitwy o zmianę grupy. (Chociaż przywróć rzeczy.) Spraw, by praca z tobą była radością.

Rób to konsekwentnie, a powinieneś stworzyć mniej konfliktów i znaleźć więcej swoich pomysłów, które zostaną przyjęte przez innych. Wszyscy cierpimy złudzeniem, że doskonały logiczny argument zachwyci innych i zwycięży dzień. A jeśli to nie działa w ten sposób, uważamy, że powinno .

Ale to jest w bezpośrednim konflikcie z rzeczywistością. Większość argumentów ginie, zanim zostanie postawiony pierwszy logiczny punkt. Nawet wśród rzekomo logicznych ludzi psychologia przebija rozum. Przekonywanie ludzi polega na przełamywaniu psychologicznych barier dla bycia właściwie słyszanym, a nie na doskonałej logice.

Czy istnieje inny argument za używaniem Konstruktorów zamiast „starego sposobu”?

„Gdy masz nowy młotek, wszystko będzie wyglądać jak gwóźdź”. - tak myślałem, kiedy czytałem twoje pytanie.

Gdybym był twoją koleżanką, zapytałbym cię „dlaczego nie zainicjować całego obiektu w konstruktorze?” W końcu taka jest jego funkcjonalność. Dlaczego warto korzystać z wzorca konstruktora (lub innego wzoru)?

Czy zaproponowana przeze mnie zmiana naprawdę jest tego warta?

Trudno powiedzieć na podstawie tego małego fragmentu kodu. Może tak - ty i twoi znajomi musicie to ocenić.

Czy masz jakieś wskazówki, jak lepiej przedstawiać takie argumenty, opowiadając się za próbą czegoś nowego?

Tak, przedyskutuj więcej na ten temat. Uzbrój się w dobre argumenty i uzasadnienia przed przystąpieniem do dyskusji.

Byłem w sytuacji, w której zostałem zatrudniony ze względu na swoje umiejętności. Kiedy zobaczyłem kod, cóż ... to było więcej niż okropne. Kiedy następnie próbuje to oczyścić, ktoś, kto był tam dłużej, zawsze tłumi zmiany, ponieważ nie widzi korzyści. Brzmi jak coś, co opisujesz. Skończyło się to tym, że porzuciłem tę pozycję i mogę teraz ewoluować znacznie lepiej w firmie, która ma lepszą praktykę.

Nie sądzę, że powinieneś po prostu grać razem z innymi w zespole, ponieważ byli tam dłużej. Jeśli zauważysz, że członkowie twojego zespołu stosują złe praktyki w projektach, w których pracujesz, to musisz im to naśladować. Jeśli nie, to nie jesteś bardzo cennym zasobem. Jeśli ciągle powtarzasz te rzeczy, ale nikt nie słucha, poproś kierownictwo o zmianę lub zmianę pracy. Bardzo ważne jest, aby nie pozwolić sobie na dostosowanie się do złych praktyk.

Do rzeczywistego pytania

Dlaczego warto korzystać z niezmiennych obiektów? Ponieważ wiesz, z czym masz do czynienia.

Załóżmy, że masz przekazywane połączenie db, które umożliwia zmianę hosta za pomocą programu ustawiającego, a następnie nie wiesz, jakie to połączenie. Będąc niezmiennym, to wiesz.

Powiedzmy jednak, że masz strukturę danych, którą można odzyskać z trwałości, a następnie ponownie utrwalić przy użyciu nowych danych, to ma sens, że ta struktura nie jest niezmienna. To ten sam byt, ale wartości mogą ulec zmianie. Zamiast tego używaj niezmiennych obiektów wartości jako argumentów.

To, na czym koncentrujesz się, to jednak wzorzec konstruktora, aby pozbyć się zależności w konstruktorze. Mówię temu duży tłuszcz NIE . Instancja jest już oczywiście skomplikowana. Nie próbuj tego ukryć, napraw to!

Tl; dr

Usunięcie seterów może być prawidłowe, w zależności od klasy. Wzorzec konstruktora nie rozwiązuje problemu, tylko go ukrywa. (Brud pod dywanem nadal istnieje, nawet jeśli go nie widać)

Chociaż wszystkie pozostałe odpowiedzi są bardzo przydatne (bardziej przydatne niż moje), istnieje właściwość konstruktorów, o której jeszcze nie wspomniałeś: przekształcając tworzenie obiektu w proces, możesz egzekwować złożone ograniczenia logiczne.

Możesz na przykład nakazać, aby niektóre pola były ustawione razem lub w określonej kolejności. W zależności od tych decyzji możesz nawet zwrócić inną implementację obiektu docelowego.

// business objects

interface CharRange {
   public char getCharacter();
   public int getFrom();
   public int getTo();
}

class MultiCharRange implements CharRange {
   final char ch;
   final int from;
   final int to;

   public char getCharacter() { return ch; }
   public int getFrom() { return from; }
   public int getTo() { return to; }
}

class SingleCharRange implements CharRange {
   final char ch;
   final int position;

   public char getCharacter() { return ch; }
   public int getFrom() { return position; }
   public int getTo() { return position; }
}

// builders

class Builder1 {
   public Builder2 character(char ch) {
      return new Builder2(ch);
   }
}

class Builder2 {
   final char ch;
   int from;
   int to;

   public Builder2 range(int from, int to) {
      if (from > to) throw new IllegalArgumentException("from > to");
      this.from = from;
      this.to = to;
      return this;
   }

   public Builder2 zeroStartRange(int to) {
      this.from = 0;
      this.to = to;
      return this;
   }

   public CharRange build() {
      if (from == to)
         return new SingleCharRange(ch,from);
      else 
         return new MultiCharRange(ch,from,to);
   }
}

Jest to noddy przykład, który nie ma większego sensu, ale pokazuje wszystkie trzy właściwości: postać jest zawsze ustawiana jako pierwsza, limity zasięgu są zawsze ustalane i sprawdzane razem, a Ty wybierasz swoją implementację w czasie kompilacji.

Łatwo jest zobaczyć, w jaki sposób może to prowadzić do bardziej czytelnego i niezawodnego kodu użytkownika, ale jak widać, ma to pewną wadę: mnóstwo kodu budowniczego (a nawet pominąłem konstruktory, aby skrócić fragmenty). Próbujesz więc obliczyć kompromis, zadając pytania takie jak:

• Czy tworzymy obiekt tylko z jednego czy dwóch miejsc? Czy to się może zmienić?
• Może powinniśmy egzekwować te ograniczenia, przekształcając oryginalny obiekt w kompozycję mniejszych obiektów?
• Czy w końcu przekazalibyśmy konstruktora od metody do metody?
• Czy zamiast tego moglibyśmy użyć statycznej metody fabrycznej?
• Czy to część zewnętrznego interfejsu API, który musi być tak niezawodny i zręczny, jak to możliwe?
• W przybliżeniu, ile z naszych obecnych zaległości błędów można by uniknąć, gdybyśmy mieli konstruktorów?
• Ile zaoszczędzilibyśmy pisania dodatkowej dokumentacji?

Twoi koledzy po prostu zdecydowali, że kompromis nie będzie korzystny. Powinieneś omówić przyczyny otwarcie i szczerze, najlepiej bez uciekania się do abstrakcyjnych zasad, ale koncentrując się na praktycznych implikacjach tego lub innego rozwiązania.

Wygląda na to, że ta klasa jest w rzeczywistości więcej niż jedną klasą ułożoną w tej samej definicji pliku / klasy. Jeśli jest to DTO, powinno być możliwe utworzenie go za jednym zamachem i niezmienność. Jeśli nie wykorzystasz wszystkich jego pól / właściwości w jednej transakcji, prawie na pewno łamie zasadę pojedynczej odpowiedzialności. Może to pomóc w podziale na mniejsze, bardziej spójne, niezmienne klasy DTO. Zastanów się nad czytaniem Czystego Kodu, jeśli jeszcze tego nie zrobiłeś, aby lepiej wyartykułować problemy i zalety wprowadzenia zmian.

Nie sądzę, żebyś mógł projektować kod na tym poziomie przez komitet, chyba że dosłownie programujesz w tłumie (nie brzmi to tak, jakbyś był w tego rodzaju zespole), więc myślę, że można wprowadzić zmiany w oparciu o twoje najlepszy osąd, a następnie weź go na podbródek, jeśli okaże się, że źle go oceniłeś.

Jednak dopóki jesteś w stanie sformułować potencjalne problemy z obecnym kodem, możesz nadal dyskutować, że konieczne jest rozwiązanie , nawet jeśli twoje nie jest zaakceptowane. Ludzie mają wtedy swobodę oferowania alternatyw.

„ Mocne opinie, słabo podtrzymane ” to dobra zasada - idź dalej z pewnością siebie, opierając się na tym, co wiesz, ale jeśli znajdziesz jakieś nowe informacje, które ci się sprzeciwiają, bądź skromny i ustępuj i włącz się w dyskusję na temat tego, jakie właściwe rozwiązanie powinno być. Jedyną złą odpowiedzią jest pozostawienie go gorszego.