Obiektowe pisanie JavaScript

Chcę nauczyć się pisać obiektowo w JavaScript i w tym celu zacząłem refraktoryzować swój projekt.

Zbudowałem taką strukturę

	$(document).ready(function () {
 
            HomeController.init('#single-post-container')
 
        });

var HomeController = function (postService) {
 
    var init = function (container) {
 
            $(container).on('click', '.js-remove-post', removePost)
	    $(container).on('submit', '#add-new-comment-form', submitAddNewCommentForm)
            //...
    };

    var submitAddNewCommentForm = function (e) {

        currentActionPostId = $(e.currentTarget).data('post-id');
        var targetFormId = 'form#' + e.currentTarget.id;

        postService.addNewComment(targetFormId, addNewCommentSuccess, fail);
    };

    var addNewCommentSuccess = function (response) {

        appendCommentsToPost(response.responseText);

        var commentsPageIndex = 1;
        saveCommentsPageIndexInSession(commentsPageIndex);

        manageCommentNavigationButtons(commentsPageIndex, response.responseText);
    };
 
    var removePost = function (e) {
        //odzywam się do postService i usuwam post
    };
 
 
    return {
        init: init
    }
 
}(PostService);

var PostService = function () {

    var addNewComment = function (targetFormId, addNewCommentSuccess, fail) {

        $.ajax({
            type: 'POST',
            url: '/Home/AddNewComment',
            dataType: 'html',
            data: $(targetFormId).serialize(),
            complete: function (response) {

                addNewCommentSuccess(response)
            },
            error: function () {
                fail()
            }
        });
    };

    return {
        removePost: removePost,
        addNewComment: addNewComment,
	//...
    }
}();

Przyjąłem taką konwencje jak widać powyżej ale mam wątpliwości czy dobrze ogarniam obsłużenie wysłania formularza (ajaxComplete). Bo w moim odczuciu wygląda to średnio. Czy da się to zrobić jakoś lepiej ?

Generalnie jak widać nie używam już eventów w HTML tylko robię to w js. Mam mieszane co do tego uczucia ale z tego co przeczytałem to best practice jest żeby html pozostał czysty.

Ale dlaczego próbujesz konkretny request obsłużyć globalnym ajaxComplete?

Strasznie to nieczytelne i przekombinowane - dodawanie postów gdzieś nie wiadomo gdzie w innym pliku. Zwracasz obiekt z jedną metodą init - to powinno być zrefaktoryzowane do funkcji. Ogólnie samego podejścia nie kupuję.

Tak jak @dzek69 pisze - onAddNewPostFormComplete powinno być wywołane tam gdzie dodajesz posta.

Część elementów przekazujesz do funkcji (swoją drogą nazwa zmiennej sugeruje, że przekazujesz obiekt, a przekazujesz tylko selektor), a część hardkodujesz - zastanów się co chcesz osiągnąć.
Niejednolity zapis funkcji - raz function expression a raz function statement - zdecyduj się.
Złe formatowanie kodu.
No i to jQuery całkiem niepotrzebne.

dzek69 napisał(a):

Ale dlaczego próbujesz konkretny request obsłużyć globalnym ajaxComplete?
Masz racje, nie pomyślałem jak można zrobić to inaczej także zrobiłem to już w normalny sposób.
W skrócie mój kod wygląda i działa w taki sposób:

	$(document).ready(function () {
 
            HomeController.init('#single-post-container')
 
        });

var HomeController = function (postService) {
 
    var init = function (container) {
 
            $(container).on('click', '.js-remove-post', removePost)
	    $(container).on('submit', '#add-new-comment-form', submitAddNewCommentForm)
            //...
    };

    var submitAddNewCommentForm = function (e) {

        currentActionPostId = $(e.currentTarget).data('post-id');
        var targetFormId = 'form#' + e.currentTarget.id;

        postService.addNewComment(targetFormId, addNewCommentSuccess, fail);
    };

    var addNewCommentSuccess = function (response) {

        appendCommentsToPost(response.responseText);

        var commentsPageIndex = 1;
        saveCommentsPageIndexInSession(commentsPageIndex);

        manageCommentNavigationButtons(commentsPageIndex, response.responseText);
    };
 
    var removePost = function (e) {
        //odzywam się do postService i usuwam post
    };
 
 
    return {
        init: init
    }
 
}(PostService);

var PostService = function () {

    var addNewComment = function (targetFormId, addNewCommentSuccess, fail) {

        $.ajax({
            type: 'POST',
            url: '/Home/AddNewComment',
            dataType: 'html',
            data: $(targetFormId).serialize(),
            complete: function (response) {

                addNewCommentSuccess(response)
            },
            error: function () {
                fail()
            }
        });
    };

    return {
        removePost: removePost,
        addNewComment: addNewComment,
	//...
    }
}();

Moim zdaniem jest to fajnie zrobione, jasno oddzielone od siebie pliki, wiadomo gdzie co jest. W serwisach robię tylko żądanie do serwera tak jak to jest w OOA. W kontrolerach mam resztę. Proszę o przykład jak można zrobić to lepiej ?
Tak jak napisałem w pierwszym poście pozbywam się eventów z tagów HTML. Zawsze ich używałem i nawet polubiłem ale chcę się nauczyć lepiej kodzić. Jakie jest Wasze zdanie na ten temat? Przejmujecie się tym, czy jest Wam wszystko jedno?

I co z HTML helpers w MVC ? Super rzecz i bardzo wygodna ale powiem szczerze, że jakoś lepiej się czuję jak mam tą stronę napisaną bez metod gereujących html za mnie. W sumie nie wiem czy to jest jakiś "błąd" że raz użyje metody pomocniczej a raz nie, czy trzeba być konsekwentnym i jak decyduje się na jakąś konwencję to twardo się jej trzymam ?

Jeżeli nie chcesz używać żadnego frameworka typu ReactJS, to proponuję użyć pewnego wzorca.

1. Na każdej stronie do tagu body dodajesz dwa atrybuty data-controller i data-action, które powinny korespondować z Twoim backendem. Np. jeżeli jesteś na stronie danego Posta, to prawdopodobnie po stronie serwera obsługuje to jakaś akcja showAction w controllerze PostsController, więcej odpowiednio ustawiasz atrybuty body.

2. Na document ready podpięty jest jeden handler. Odczytuje on atrybuty z tagu body i przekazuje je do dispatchera, po czym wywołuje metodę dispatch.

// plik główny app.js
import Dispatcher from './dispatcher';

const body = document.body,
    controller = body.getAttribute("data-controller"),
    action = body.getAttribute("data-action");

jQuery(document).ready(() => {
    const dispatcher = new Dispatcher(controller, action);
    dispatcher.dispatch();
});

3. W dispatcherze sprawdzamy, że zaimportowane controllery zawierają controller common, czyli controller do którego pchasz wszystko, co ma się odpalić na każdej stronie, np. jakieś dirty fixy czy cos.
   Później sprawdzane jest czy istnieje taki controller jak zadeklarowałeś w tagu body i wywoływana jest na nim akcja. Dzieki temu badzo szybko jesteś w stanie zlokalizować kod, który odpala się na danej podstronie. 

// plikt dispatcher,js
import * as controllers from './controllers/all';

const COMMON = 'common';

export default class Dispatcher {

    constructor(controller, action = 'init') {
        this.controller = controller;
        this.action = action;
    }

    dispatch() {
        if(typeof controllers[COMMON] === 'function') {
            controllers[COMMON]();
        }

        if(this.controller in controllers) {
            const controller = new controllers[this.controller]();
            typeof controller[this.action] === 'function' && controller[this.action]();
        }
    }
}

// controllers/all.js
export {default as common} from './common';
export {default as QuestionsController} from './questions'

// Przykład common.js
import setupToastr from '../includes/toastr';
import initWelcomeTour from '../includes/welcome-tour'
import {bindAjaxButtons, facebookLoginFix} from '../includes/utils'
import {bindFacebookLogin} from '../includes/login'

import {
    bindFacebookLikeEvent,
    bindFacebookShareEvent,
    bindTwitterShareEvent,
    bindWykopShareEvent
} from '../includes/social-shares';

const common = () => {
    facebookLoginFix();

    // Styling issue
    $('nav a').click((e) => $(e.currentTarget).blur());

    $('.selectpicker').selectpicker({
        width: '250px'
    });

    setupToastr();

    window.fbAsyncInit = function() {
        bindFacebookLikeEvent();
        bindFacebookShareEvent();
    };

    window.twttr.ready(function() {
        bindTwitterShareEvent();
    });

    bindWykopShareEvent();
    bindAjaxButtons();
    bindFacebookLogin();

    initWelcomeTour();
};

export default common;

// przykład innego controllera
import {bindSubmitAnswerHandler} from '../includes/questions-feed'
import {bindDynamicAnswers} from '../includes/questions'

export default class QuestionsController {

    constructor() {
    }

    index() {
        bindSubmitAnswerHandler();
    }

    show() {
        bindSubmitAnswerHandler();
    }

    create() {
        bindDynamicAnswers();
    }
}

Całe mięcho kodu siedzi w katalogu includes (akurat mi to wystarczy), controller tylko odpala dany kod.
Czyli podsumowując potrzebujesz plików:

• app.js (podany wyżej)
• dispatcher.js (podany wyżej)
• katalog controllers na controllery
• katalog includes, czy dowolną inną strukturę do organizacji pozostałego kodu, który będziesz importował do controllerów.

PS. Nie używaj jakiś async:false czy coś, tylko użyj javascript promise.

@Desu
Nie wnikając w samą koncepcję:

export default class Dispatcher {
 
    constructor(controller, action = 'init') {
    }
 
    dispatch() {
    }
}

jest bezsensowne, o ile nie piszesz w Javie (gdzie inaczej się nie da) powinno to byc zrefaktoryzowane do funkcji:

const dispatch = (controller, action = 'init') => {
};

Każda klasa z jedną metodą (lub metodą + konstruktor) to upośledzona funkcja z niepotrzebnym narzutem zmniejszającym czytelność i wydłużającym kod.

Polecam:

http://steve-yegge.blogspot.com/2006/03/execution-in-kingdom-of-nouns.html

Słuszna uwaga i dzięki za linki. A co myślisz o samej koncepcji takiego podziału kodu? Ostatnio wprowadziłem to do projektu, w którym miał być sam jQuery (bo o taki usecase chodzi, gdzie nie ma angulara, reacta itp), a do tej pory kod był porozrzucany w widokach (na samym dole w tagu script).

@Desu
Podstawowy zarzut do Twojego kodu to to, że ładujesz do każdego widoku wszystkie kontrolery, mimo że potrzebny Ci tylko jeden (swoją drogą korzystanie z wildcard import * to zły pomysł, wszystko powinno być jawne). Ba - nawet wygląda na to, że potrzebne Ci tylko pojedyncze akcje, więc akcje moga być odrębnymi funkcjami, nie ma potrzeby upychać ich w klasie i incjalizować całość.

Druga rzecz to struktura katalogów - dużo lepszym pomysłem wydaje się dzielenie katalogów zgodnie z częściami aplikacji a nie według typu pliku (controller, view etc.), np:

/shared (lub commons, obojętnie)
/utils
/home
/articles
/article
    index.js (tu importujesz commons jeśli potrzebne oraz funkcjonalności konkretnego widoku, np wczytujesz bezpośrednio potrzebną akcję - to twoj "kontroler")
    ... (reszta plików specyficznych dla widoku, w razie potrzeby pogrupowane w podkatalogach)
...

Przy powyższym podejściu coś takiego jak dispatch w takiej formie jak zaproponowałeś przestaje być potrzebne.

@Maciej Cąderek w takim razie w jaki sposób pisać te javascripty ? Na pewno jest jakiś wzorzec który można stosować dla 'crudowych' aplikacji.

Czesc, podepne sie do watku. Czy pisanie pluginow rowniez tym sposobem sie oplaca?

var addNewCommentSuccess = function (response) {

niepotrzebne ci tu var. Lepszym rozwiązaniem w tym przypadku jest zadeklarowanie funkcji (zamiast robienia zmiennej):

function addNewCommentSuccess (response) {

1. Krócej, bardziej przejrzyście

2. Nazwę funkcji widać będzie w Dev Toolsach w przeglądarce, co może ułatwić debugowanie. Teraz niby nazywasz funkcję, ale dla przeglądarki ta funkcja jest anonimowa (tj. funkcje anonimowe nie są złe, ale jak już chcesz nazwać funkcję, to przykro mi, ale wcale jej nie nazwałeś).

W ES6 doszły jeszcze arrow functions, które często są czytelniejsze (ale tego też na oślep się nie stosuje, arrow functions też mają wiele ograniczeń).

Chcę nauczyć się pisać obiektowo w JavaScript i w tym celu zacząłem refraktoryzować swój projekt.

Czy programowałeś wcześniej w Javie?
Pytam, bo dość przekombinowane masz to nazewnictwo, nie można nic zrozumieć, np.

manageCommentNavigationButtons

trochę WTF.
i same takie dziwne nazwy.
chociaż nazewnictwo to jedna sprawa (może się orientujesz w tym, nie wiem), gorzej, że masz dość zaplątany przepływ sterowania, np.:
masz funkcję init, funkcja ta ustawia przez jQuery callbacka submitAddNewCommentForm, kiedy ten callback się wykona, wywoła się funkcja postService.addNewComment z callbackiem addNewCommentSuccess, a ten ostatni callback wywołuje jakieś z d... wziętą funkcję appendCommentsToPost i parę innych.

Takie latanie flowem programu (szczególnie programując asynchronicznie) to prosty przepis na katastrofę, race conditions, bugi, czy WTFy (po tygodniu sam nie będziesz pamiętał która funkcja wywołuje którą). Być może użycie promise'ów byłoby tu rozwiązaniem. Bo tak, to się robi z tego zwykły spaghetti kod.

No i mam wrażenie, ze z połowa tych funkcji ci w ogóle niepotrzebna (ale nie jestem tego w stanie stwierdzić, bo nie wrzuciłeś całego kodu a tego kawałka nie da się za bardzo zrozumieć ).

Może opisz co Twoja aplikacja ma robić, jaki jest podział zadań między backendem a frontem, jak dużo dzieje się na froncie, jak się komunikujesz z backendem (z tego co widze przesyłasz ajaksem html??). Mam podobne odczucia jak

@Maciej Cąderek aplikacja typowo backendowa jeżeli mogę tak powiedzieć. To co się dzieje na frontendzie to właśnie sprawdzanie czegoś asynchronicznie na podstawie jakiś danych -> zwrócenie wyniku, zapis czegoś do bazy, pobranie z bazy (wszystko ajax), no i właśnie zwrócenie jakiegoś widoku częściowego i podmienienie danych.

@LukeJL dzięki za objasnienie.

Czy programowałeś wcześniej w Javie?
Pytam, bo dość przekombinowane masz to nazewnictwo, nie można nic zrozumieć, np.
manageCommentNavigationButtons

Programuje w C# . Fakt faktem nie mam dużego doświadczenia zawodowego, nie jestem mistrzem nazewnictwa, często mam z tym problem, a jak już coś wymyśle w miarę sensownego to nie jestem często z tego zadowolony. Co do funckji manageCommentNavigationButtons jest to funkacja która zarządza przyciskami do nawigacji w komentarzach (prev, next). Sprawdza czy przyciski mają być dostępne czy nie i jak nie to je ukrywa, a jak tak to pokazuje. Jak byś to lepiej nazwał, bo jeżeli ta nazwa jest beznadziejna to ja się aż boje o moje niektóre hardkorowe nazwy.. ?

masz funkcję init, funkcja ta ustawia przez jQuery callbacka submitAddNewCommentForm, kiedy ten callback się wykona, wywoła się funkcja postService.addNewComment z callbackiem addNewCommentSuccess, a ten ostatni callback wywołuje jakieś z d... wziętą funkcję appendCommentsToPost i parę innych.

Czemu z d**y funkcję ;p ?
Po dodaniu komentarza do bazy wywołuje się szereg funkcji która temu towarzyszy.
appendCommentsToPost - wyświetlam w tym przypadku pierwszą stronę komentarzy (żeby było widać komentarz który dodał użytkownik, mógł być na innej stornie).

var commentsPageIndex = 1;
saveCommentsPageIndexInSession(commentsPageIndex) - to jest funkcja związana z tym że każdy post może mieć rozwinięte komentarze ( tak jak na facebook i ja muszę wiedzieć jak coś kliknę do którego postu są komentarze)

manageCommentNavigationButtons(commentsPageIndex, response.responseText) - to już wytłumaczyłem..

Wiem, że jak ja coś muszę tłumaczyć bo z mojego kodu nie rozumiesz czegoś to jest to strasznie słabe, dlatego proszę podaj przykład na co byś to zmienił, wtedy bym się czegoś nauczył.

Takie latanie flowem programu (szczególnie programując asynchronicznie) to prosty przepis na katastrofę, race conditions, bugi, czy WTFy (po tygodniu sam nie będziesz pamiętał która funkcja wywołuje którą). Być może użycie promise'ów byłoby tu rozwiązaniem. Bo tak, to się robi z tego zwykły spaghetti kod.

No w moich oczach też jest to trochę problematyczne, ale mi się spodobało, a nauczyłem się tego podejścia na jednym z kursów pluralsight więc uznałem że jest git. (również dotyczyło aplikacji podobnego typu)

No i mam wrażenie, ze z połowa tych funkcji ci w ogóle niepotrzebna (ale nie jestem tego w stanie stwierdzić, bo nie wrzuciłeś całego kodu a tego kawałka nie da się za bardzo zrozumieć ).

Po czym wnosisz ;> ?

a zamiast:

$(document).ready(function() {
})

krócej:

$(function() {
})

tak samo widzę często przy dodawaniu:

type="text/javascript"

przecież bez tego i tak przeglądarki wiedzą co mają robić, a te co nie wiedzą i tak będą błędnie wyświetlały skrypty więc...

@RideorDie

Przeanalizowałem lepiej Twój kod - samo podejście nie jest złe, ale:

• mieszasz obsługę postów i komentarzy,
• zasięg zmiennych kuleje - wrzucasz callbacki dla eventów do ogólnego scopa kontrolera,
• funkcje używane w success (np. manageCommentNavigationButtons, appendCommentsToPost itd.) to w ogole jakieś globale nie wiadomo skąd - pomijam,
• uzywasz callbacków zamiast deffered / promise co mocno gmatwa kod,
• akcje na backendzie jakieś "mało" RESTowe są, a powinny być,
• przerzucasz do serwisów pobieranie potrzebnych danych, przez co w dwóch miejscach operujesz na DOMie - lepiej przekazać im tylko to czego potrzebują,
• uzywasz atrybótów class z prefiksem js- - klasy nie są są do takich rzeczy, lepiej operowac na atrybutach data-,
• uzywasz funkcji przed ich deklaracją (co w tym wypadku zadziała bo to callbacki, ale nie jest to zbyt fajne).

Nie pisze raczej klasycznych aplikacji (raczej SPA, a ostatnio to w ogole backend w Node.js), nie używam też ES5 ani jQuery, mogłem tez nie zrozumiec do końca o co Ci chodzi (słaby opis) ale na szybko to zrobiłbym to w mniej wiecej taki sposób:

• struktura katalogu (oczywiście można pgrupować w podkatalogi typu posts, comments jesli bedzie tego więcej):

/home
  index.js
  posts.js
  postsService.js
  comments.js
  commentsService.js

• index.js:

var modules = [
  posts,
  comments
];

modules.forEach(function(module) {
  module.init();
});

// lub jak jest tego niewiele to ręcznie:
posts.init();
comments.init();

• posts.js:

var posts = function (service) {

  function init(container) {
    $(container).on('click', '[data-action="remove-post"]', removePost)
    //...
  };

  function removePost() {
    service
      .remove(this.dataset.id)
      .done(function (response) {
        // do something with response
      })
      .fail(function () {
        // error handling
      });
  };

  // ...

  return {
    init: init
  }
}(postsService);

• comments.js:

var comments = function (service) {

  function init(container) {
    $(container).on('submit', '[data-action="add-new-comment"]', addNewComment)
    //...
  };

  function addNewComment() {
    var data = $(this).serialize();
    
    service
      .create(data)
      .done(function(response) {
        // do something with response
      })
      .fail(function () {
        // error handling
      });
  };

  // ...

  return {
    init: init
  }
}(commentsService);

• postsService.js:

var postService = {
  remove: function (id) {
    return $.ajax({
      type: 'DELETE',
      url: '/posts/' + id
    });
  },
  // ...
};

• commentsService.js:

var commentsService = {
  create: function (data) {
    return $.ajax({
      type: 'POST',
      url: '/post-comments'
    });
  },
  // ...
}

No i napisze jeszcze raz:

• przerzuć się na ES6,
• zamiast jQuery.ajax lepiej uzyć lżejszego, w nowych przeglądarkach już natywnego API uzywającego prawdziwych promisów: https://github.com/github/fetch ( https://developer.mozilla.org/en/docs/Web/API/Fetch_API )

PS
Jak będzie mi się chciało to moze sklecę przykład w ES6 + fetch API