Wzorzec MVC na przykładzie prostego CRUD-a - czy dobrze to zaimplementowałem?

0

Siema.
Chciałem ogarnąć sobie wzorzec MVC, więc piszę prostą apke TODO-list.

Rozdzieliłem kod JS'a na trzy pliki - to są na razie tylko szkielety klas, chodzi mi o sam zamysł:

Model: https://pastebin.com/HCwpz9Yz

View: https://pastebin.com/bNPqezN1

Controller: https://pastebin.com/8SXGgA6M

Tu się pojawia moje pytanie, czy dobrze to podzieliłem?
Jeżeli nie to co musiałbym poprawić?
Prosiłbym jeszcze o jakiś pomysł jak powinienem zaimplementować obsługę kontrolera z pliku html, bo na razie skleiłem coś takiego:

<script type="module">
        import app from "./todo-controller.js";
        document.getElementById("projects-btn").addEventListener("click", () => {
            app.expandProjectsMenu();
        });
    </script>

ale nie wydaje mi się to zbyt estetyczne.

1

addEventListener przenieś do widoku. No i raczej widok powinien być na przodzie, a w tym HTML możesz sobie ewentualnie zrobić mały glue code:

    <script type="module">
        import {TodoView} from "./todo.js";
        const root = document.getElementById('todo');
        const view = new TodoView(root);
    </script>

W ten sposób odetniesz go od globalnego stanu, będzie grzebał tylko w tym kawałku DOM, który mu podrzucisz.

Poza tym jest funkcja createTask, która robi, to co powinien robić ewentualnie konstruktor. Jest klasa TasksList, która zachowuje się jak mapa (może wystarczy zwykły array?).

Stylowanie możesz zmienić na:
gear_img.style.transform = "rotate(0deg)"

No i snake_case.

2

Tu się pojawia moje pytanie, czy dobrze to podzieliłem?

Okaże się w praniu. Przy tak małym kawałku kodu (~100 linijek kodu), to nawet jakbyś miał to napakowane w jednym pliku i nawet bez zachowania żadnych większych zasad, nie byłoby szkody. Bo ogólnie mam wrażenie, że z tego kodu to niewiele za bardzo wynika i jest podzielony za wcześnie, zanim w ogóle się zdecydujesz, co chcesz tak naprawdę zrobić. Chyba, że to część większej całości, ale właśnie wygląda jak coś bardzo surowego, co nic nie robi, a tylko pachnie.

to są na razie tylko szkielety klas

protip: najlepiej jak coś się robi za pierwszym razem, to robić mięsko, pisać prosty kod, który robi, co ma robić, nie bawić się w jakieś szkielety i wysublimowane wzorce. Dopiero potem, jak już wiesz, co chcesz zrobić (plus jakie problemy mogą wyniknąć po drodze), to wtedy można się pobawić w podejście top-down.

if(TodoController.instance)
     return TodoController.instance;
TodoController.instance = this;

po co ten singleton? Nic ci to nie daje. Jak chcesz mieć jeden obiekt, to możesz po prostu zrobić tak:

const app = {
        model: new TodoModel(),
        view: new TodoView(),
        openAbout(){
           this.view.openAbout();
         },
         expandProjectsMenu(){
           this.view.expandProjectsMenu();
         }
};

to samo w tym przypadku, co osiągasz z singletonem, ale krócej.

Ale z drugiej strony, czemu zakładasz, że kontroler ma być tylko jeden (i ten jedyny kontroler będzie miał 1 model i 1 widok)? A co jeśli będziesz chciał mieć ileś modeli i widoków naraz?

 this.projects_list = new Map();
 this.addProject("default");

zdecyduj się, albo camelCase albo snake_case (a w przypadku JSa ogólnie przyjętym standardem jest camelCase). Poza tym jak list, to już nie musisz robić liczby mnogiej (lista projektów = projectList).

 for(const property in task_data)

To z const ci działa? Wow. Ja zawsze z let robiłem.

No ale const ma nawet sens, bo jeśli wiązanie property jest robione dla każdej nowej iteracji, to mimo, że wartość property będzie ciągle inna, to jednak będzie to ciągle nowa(!) zmienna, więc nie będzie przypisania na nowo, więc faktycznie może być const (tzn. chyba to tak działa. kiedyś to próbowałem zrozumieć dokładniej czytając specyfikację, ale specyfikacja JavaScriptu to nie jest przystępna lektura. Chociaż w nieco bardziej przystępny sposób to tłumaczy książka You don't know JS: https://github.com/getify/You[...]a/scope-closures/ch5.md#loops )

0

@LukeJL:

...najlepiej jak coś się robi za pierwszym razem, to robić mięsko, pisać prosty kod, który robi, co ma robić, nie bawić się w jakieś szkielety i wysublimowane wzorce

Myślałem, że najlepiej od początku trzymać się wzorca żeby później nie bawić się w zbędną refaktoryzację wielkiego kloca kodu ;d

Ale z drugiej strony, czemu zakładasz, że kontroler ma być tylko jeden

Wydaje mi się to logiczne jedna klasa która "zarządza" całą stroną, nie widzę zastosowania dla większej ilości kontrolerów, mógłbyś podać jakiś przykład jakby to mogło wyglądać, niekoniecznie w przypadku mojego kodu?

0
Eldorad O. napisał(a):

@LukeJL:

...najlepiej jak coś się robi za pierwszym razem, to robić mięsko, pisać prosty kod, który robi, co ma robić, nie bawić się w jakieś szkielety i wysublimowane wzorce

żeby później nie bawić się w zbędną refaktoryzację wielkiego kloca kodu ;d

Co złego w refaktoryzacji? ;) Poza tym nie musi to być wielki kloc kodu. A małe kawałki kodu można przepisać od zera.

Myślałem, że najlepiej od początku trzymać się wzorca

Lepiej zrobić coś dobrze, a nie "trzymać się wzorca". A jak masz zrobić coś dobrze za pierwszym razem? O wiele łatwiej jest zrobić coś dobrze, robiąc coś najpierw źle (żeby zobaczyć, jak podejść do problemu i sprawdzić, co jest dobrym pomysłem, a co niezbyt trafionym), a potem dopiero przepisać na czysto. Zarówno jest chodzi o funkcjonalności, jak i techniki. Za pierwszym razem też nie zaimplementujesz dobrze wzorca. Najlepiej pierwsze ileś podejść traktować jak wprawki i prototypy.

Wydaje mi się to logiczne jedna klasa która "zarządza" całą stroną, nie widzę zastosowania dla większej ilości kontrolerów, mógłbyś podać jakiś przykład jakby to mogło wyglądać, niekoniecznie w przypadku mojego kodu?

A co jeśli będziesz chciał użytkownikowi pokazać nie jedną, a dwie (albo 5) list todo naraz? I każda taka lista musiałaby być niezależnie sterowana przez użytkownika oraz mieć osobny model.

A jeden na stronę to mógłby być faktycznie np. moduł do łączenia się z serwerowym API

1 użytkowników online, w tym zalogowanych: 0, gości: 1, botów: 0