search

alpheios-project / documentation

Alpheios Developer Documentation
0 stars 0 forks source link

Component (Object) Architecture #3

Closed kirlat closed 4 years ago

kirlat commented 5 years ago

@balmas, @irina060981: Please review a proposition of an application architecture for our business classes: https://github.com/alpheios-project/documentation/blob/master/development/component-architecture.md. I think it would be beneficial to us to standardize behavior of such components as UIController, ContentProcess, BackgroundProcess, etc.

Decided to assemble this piece and offer if for discussion before doing changes to the controller objects required to avoid preliminary data loading in Safari tabs, when content script is loaded to every tab automatically.

Please let me know your opinions. It would be great if we combine our experiences and ideas on this! Thanks!

balmas commented 5 years ago

I think this is a great start at defining architectural best practices for us to follow, and providing a template for business components.

I also really like the idea of isolating business logic from the display. I am becoming a little uneasy with the depth of our dependency on the Vue framework, and decoupling the business logic from the display logic will also help us to not become too dependent upon any one framework.

irina060981 commented 5 years ago

I agree with Bridget - it is a great start for creating a higher-level application :)

I have some thoughts about this approach:

1) I really like the idea of separating Bussiness logic from Display components. According to Vue components we could divide code from there on the following groups: a) template part with sub-components, slots and different conditional rendering b) properties, methods and watchers that controlls rendering (render or not, what exactly to render) c) methods and properties that handles with current drawing, with html nodes, resizing, applying different styles d) methods and properties that inits events, changes properties for child components (and make them re-render) e) methods and properties for working with data, manipulating data, retrieving data - bussiness logic c) styles block and they can bw defined for the current template and for the child template

I think that if we want to become less Vue-dependable and be ready to be used with different frameworks - we should reduce the data in Vue components.

I think that we should use Vue components as a view templates that is only rendering input data, sending events and re-render on changing input data. I think that it would be better to remove from methods direct handling with HTML nodes and move it to some Service classes or functions outside components. This way we or someone else from community would be able to change Vue to any other templates or franework.

2) I think that we should put localization to some external outstanding library - as it could be used in different parts of the application/applications (for example inflection games could have its own localization, when we will decide to rework definitions/translations parts they could need its own localization, I think)

3) We have a mix with different styling in component, embedded-lib and inflection-games. I am adding styles for it through the link to components repo to be able to handle with changing fonts and color-schema. If we want to work with more complex styling and skin system, that could have more individual design , may be we should think about separating it from components ?

kirlat commented 5 years ago

Great ideas, Irina, thanks for sharing! Agree with every word you said.

Creating a Vue infrastructure we should also keep in mind its memory consumption. This probably is a topic for another discussion so I wont' go deep into that. A simple example could be an inflection table. Vue rendering is lazy, and Vue uses node patching. This means that if we have a rendered inflection table, it will stay in memory even if we switch to the other tab or work in a popup. It has many cells. If each cells has references to some other objects, those objects would stay in memory too. And those objects could refer to some other instances too. It could be even strong circular references. That all could eat up lots of memory! Should take those aspects into consideration when deciding on our architecture too.

For localization we have L10n, so we could probably add more functionality to it if needed and move it to become a separate library.

It's also so great that you mentioned styles! I think this is a very important part of an architecture (even though it might be orthogonal to the JS objects it does not matter). We probably need to have a different doc on styles too. I agree that it would probably make sense to separate styles from components (even though components, especially reusable ones, should probably have its own minimal styling to be of any value) but have no idea so far how it's best to be done. it's a complex topic as styles goes through a UI of several apps.

The big issue in styles is naming. We're using BEM approach, but that requires to identify and separate visual components form one another. Should pay attention to that.

We probably need to have at least two layers of styles: default ones and the ones that represent skins. We should also try to minimize rendered CSS if possible. Maybe we can peek at what large CSS frameworks (Bootstrap, UIkit, etc.) do here to borrow their expertise? Or maybe there is something even better than that?

Another interesting technology that might benefit our visual components are CSS modules: https://css-tricks.com/css-modules-part-1-need/. Maybe we should research this topic too when we can.

irina060981 commented 5 years ago

I am happy, that you liked these ideas :) About Css Modules - from my point of view the Vue approach is better than CSS Modules described in the article. All sytles in components could be created with scoped parameter and it would be rendered this way: 1) everytime you compile the Vue application - it creates an additional data parameter to every html tag (for components that has scoped styles parameter) 2) this data parameter is added to styles definitions For example, image

This parameter re-inits each compiling.

irina060981 commented 5 years ago

From my experience our approach to styling is close to used in Bootstrap:

1) there are all styles divided to separate files (less/scss) - some of them are core and should be used in any build and some of them are specific to used components. So when you want to use some custom build - you should decide and those you neeed.

2) There is a constant parameters files (colors, base-fonts, base layout sizes)

3) All files are used with these parameters.

4) So if you want to apply some skin - you are creating an additional skin class and change constants according to the skin.

In our case It seems to me that we couldn't really optimize styles amount - as they should be compiled before and append through the only css file to head. And BEM approach makes additional amount of memory that needs to save our styles (because of long classes name). For our project that uses really not so much html markup we have 287Kb styles only in components repo.

kirlat commented 5 years ago

Moved styles-related discussion to #4 to keep this issue focused on component (object) architecture.

kirlat commented 5 years ago

@balmas, @irina060981! Please review a proposal to update a business components architecture: https://github.com/alpheios-project/documentation/blob/master/development/app-architecture.md (described in Business components architecture section). Thanks!

irina060981 commented 5 years ago

@kirlat , I have read your deatiled description only now (I am sorry, it seems I have missed an email about this update). As I have written my thoughts in other issues about UIController ( https://github.com/alpheios-project/webextension/issues/130#issuecomment-433346918 https://github.com/alpheios-project/components/issues/257#issuecomment-433360241 )

I think that you are doing a great job describing all here with detailed schemas! I have only a couple of question:

1) why did you decide to include all adapters as singletone (now they have different ways to be added) ? What are the advantages of this type of connecting? (i have used different ways in my experience and have no idea what is the best one)

2) do we consider to use UIController only according to web page with our some Vue components ? (and it is natural from UIController title) For example for inflection games, lexical tests - should I create something similiar to UIController but in separate repo?

kirlat commented 5 years ago

@irina060981! Thank you so much for your detailed answers. They're great pieces of information and super helpful. I would like to mull over it a little.

I will try to answer more simple questions here :smile:.

Will start from the second one about a UI controller. I should probably travel back in time to explain it. When this project started, we were not using Vue.js yet. All visual components (and inflection tables as the most complex of them) where generating their HTML code manually. So we had components that rendered HTML of inflection tables, panel, popup. Those, in terms of Vue and similar frameworks where presentation components: the ones whose main purpose was to display something. However, we needed some controller to tie them up together. That's why UI controller was created. So if someone selected a different inflection table from a drop down, UI controller would intercept that and send a command to inflection tables library to generate and display a different table. Thus, the UI controller is something what Vuex is to Vue presentational components. Now we migrated all our presentational components into Vue SFC format, but UI controller still remained what it was. But I think once we start using Vuex, most of it's functionality, if not all, would go to Vuex. So the UI controller would not exist as a "supercontroller" what it currently is.

Also, I want to note that the purpose of a UI controller was user input coordination only, If one wants to get lexical data via a lexical query, he/she should not use UI controller at all. That's what queries are for: to get information from one or several sources. So if one wants some lexical data, there is no need for a use UI controller at all. One should start a new LexicalQuery, and it should return all lexical data obtained. If somebody wants something like annotations, he/she should use AnnotationQuery and so on. One specialized query per type of data. That was the idea. Now, unfortunately, it's not so pure: LexicalQuery is coupled heavily with a UI controller as it calls multiple methods of it in the process. But I think we could (and should) fix that.

If for your tasks you don't need any visual data display, then you don't need an instance of a UI Controller at all. You need to use Queries to get the data you need and that's it.

If you want to display some different UI components in some different way, you probably need a different UI controller for that. It can inherit from a base UI controller component, or we could use composition where a UI controller would be composed of different modules instead. But maybe we would Vuex to replace a UI controller functionality.

That's the idea. Implementation may differ from what's described, but it doesn't mean we couldn't fix it.

The answer to your first question about singletons lies in area of ownership and memory management. When something that exists in memory for a long time (like a UI controller) owns some other object that occupy a sizeable chunk of memory, that other object cannot be garbage collected. My idea was that instead of holding some adapter for a long time, it would make sense to get a link to it only when we need it (inside of a lexical query, for example) and then release it (when lexical query is done) so it can be garbage collected. And singleton pattern, as I thought, would simplify reference management. If we have a reference to a morphAdapater, for example, and we store a reference to it in a UI controller and then we need to pass it to a LexicalQuery, we have to pass it as an argument. That could become cumbersome when we need to pass several references this way. It might be beneficial just to call let morpAdpater = MorpAdapter.instance() inside a LexicalQuery and then do let morphAdapter = null when query is done so the adapter can be garbage collected. So it is, on my opinion, simpler and may reduce memory consumption. But I think it can not work in all scenarios, so I don't think all those adapters shall always be singletons. There can be other considerations: if an adapter is expensive to initialize, we might initialize it once and hold for the whole lifetime of a UI controller. We should find a right approach for each particular case. So we would probably use both approaches (reference and singleton).

What do you think? Does it make sense?

irina060981 commented 5 years ago

@kirlat, thank you for the history explanation - it is really interesting for me as for a newby :) About using Lexical Query instead of UIController - it is deeply depended with UIController not only because cross-update methods, it uses options from Storage - so these options should be got using UIController - as it has not so simple workflow for that, it uses localization from UIController it has a UIController as its own property :) So they are really couldn't be used one without another :)

I understand (thank you for the story) that it is only a step on a upgrading/developing way :)

About instances - it is not really clear for me - because - you are to pass the whole class instead of passing the object, and it will store almost the same structure for the class in the memory. May be there will be some benefit from not storing current values.

I think that we could reach some benefits if use adapters the same way as axios We import the adapter as module and init it inside the for example lexical query with some configuration stored in for example UIController or somewhere else.

Then all instance with the data of the whole class will be used inside closure and will be cleared after LexicalQuery object destroyed. And we will be able to save the configuration and some customizable ability. What do you think about this variant?

kirlat commented 5 years ago

I think we should eliminate UI Controller dependencies within LexicalQuery, if possible. Maybe we should even split lexical query functionality into several more specialized subqueries. I think it's dependence on UIController is bad.

Regarding axios, are you suggest to use something like

const instance = axios.create({
  baseURL: 'https://some-domain.com/api/',
  timeout: 1000,
  headers: {'X-Custom-Header': 'foobar'}
});

where a static method creates an instance with specified parameters? It's something close to a builder pattern that is so popular in Java. I think this can be a good approach.

balmas commented 5 years ago

Happy to see this discussion evolving!

I think ultimately we want the UIController (or whatever it ends up being called) to be where Alpheios-specific business logic that crosses components and state lives. This is business logic that we want to be accessible to and consistent across all forms of Alpheios applications. I'm not sure what portion of it should move to Vuex and what should stay in the UIController but my instinct is that Vuex is responsible for managing and sharing state across the Vue components and the UIController is responsible for describing how and when that state needs to be shared.

I think we might begin to really understand how this is needed once we add user data into the mix. But for example, the interaction between the lexical query results, the grammar, the inflection tables, annotation resources, etc. should be consistent across all applications that use Alpheios. Not all applications should be required to have all of these resources, but for whichever of them they do have, the interaction should be consistent.

balmas commented 5 years ago

So the individual applications (Embedded library, PWA, Webextension, etc) need to be able to specify which components they want to include, which services those components should use and how they are configured, but should not have to wire them up together or have to know how they interact.

balmas commented 5 years ago

And once we have the ability to store user data, we will want to be able to keep track of user interactions across components. For example, maybe certain sequences of steps (look up a word, access inflection tables, access grammar) would be combined to form a template for a future pedagogical exercise. Or to be recorded as one complete user experience. The details of this experience may differ depending upon which application they are using, but we will want to be able to have consistency in the way it is managed.

kirlat commented 5 years ago

One of the problems we have with a UIController right now is tight coupling between its modules. For example, LexicalQuery calls multiple methods of a UIController. Because of this, LexicalQuery cannot be used independently.

To make LexicalQuery independent, we could make it return all data at the end as a return value. But this will be far from optimal. For example, this will prevent us from displaying pieces of query results as they arrive.

To solve this, we can switch to an event-based communication between components. So if something, like a UIController, wants to receive pieces of data from LexicalQuery as they arrive, it subscribes to LexicalQuery events. If LexicalQuery fetches lemma translations, for example, it will fire a LemmaTranslationsReady event. A function of a UIController that is subscribed to that will receive an event along with the lemma translations info. UI controller will be able to display that without further ado.

As a result, LexicalQuery is fully decoupled from a UIController. If nothing subscribes to a LexicalQuery events, it will still go on its own (although that'd be a wast of time, of course, but still). On the opposite end, it can have multiple components listening to events of a LexicalQuery, but this still would make no difference to LexicalQuery. It will notify all its subscribers. Also, adding or removing listening components would require no change in LexicalQuery's code.

This all can go a long way between many existing components.

@balmas, @irina060981: would that be a worthy approach? Do you see any drawbacks in it?

irina060981 commented 5 years ago

I think there could be 2 approaches (according to the discussion):

1) consider UIController as a unique controller for handling with getting different data, working with saved states and passing data to various view elements - and in this variant we should use LexicalQuery with UIController. And UIController should be very configurable and be easy to use with others.

2)UIController and LexicalQuery are independent - they could be used one without another or could be used together. And there are some very generic methods to communicate - for example using events.

The main problem with generic events is that events handling is not a standard mechanism for all used platforms - chrome and safari has some difference between using events and node version should have node-way events. It seems to me that creation of such overall bus events could be not an easy solution.

Also let's imagine in what cases we should use LexicalQuery separated from UIController - for LexicalQuery we need different options (language, vocabular preferences for definitions), need some localization for passing messages and we need to have define the output rules for different parts of recieved data. So to use all it we should duplicate the UIController sence in some separated place and it won't be an easy task for anyone.

For example, let's imagine we need to use LexicalQuery for getting data for Inflection-games, I have as an input some word (may be with additional defining data) and want to get from LQ inflection-tables and lemmas with part of speech and definitions. I should pass:

It seems to me that I will need to recreate a big part of UIController and also I will need to know a lot of details how to create it.

In this way I would prefer to use some ready UIController solution but configurable and independent from any view components with well documented input and output. This way it could have an easier integration to different tasks (browser, embedded, node version).

irina060981 commented 5 years ago

I think that bus events solution is a good way to use (I saw the big Kyle Simpsons speech about using events to build comunication). But it needs some additional solution for unifying events.

kirlat commented 5 years ago

Agree with you that there is no standard event handling mechanism we can use. That's why I was thinking we can create our own based on the Reactor pattern. This is pretty simple, and we won't be dependent on any platform-specific implementations. Here is one of the examples of how this can be done: https://stackoverflow.com/questions/15308371/custom-events-model-without-using-dom-events-in-javascript

You've raised some very good points about LQ and UIC roles in the application. I need to think about it, and we'll write my opinion on this later.

balmas commented 5 years ago

I like the event-based approach, and agree that, with some care, we can use a solution like the Reactor pattern so that LexicalQuery can be used without DOM events.

As Irina mentioned, it's a little difficult to know where the line is between the UIController business logic and the LexicalQuery business logic. We want consistent behavior of lookups, particularly with regard to language-specific variations and user preferences (See alpheios-project/components#269 for an example of a language-specific refinement that is needed).

I think UIController should be responsible for business logic of the visual components and state of the application, and the LexicalQuery should be responsible for business logic of the queries (in fact, it might be better renamed QueryController) and maybe that distinction will help?

kirlat commented 5 years ago

I agree that LexicalQuery (and QueryControllers) shall be responsible for obtaining data only. That data shall be displayed by a UIController, which, in turn, would be responsible for presentation.

In that schema, a LexicalQuery should not generate any messages that will be displayed in the UI nor it should decide whether to do something on the UI side. That's responsibility of a UI Controller. A LexicalQuery should just obtain data correctly and once each new piece of data become available, it should pass that data piece to a UI Controller.

Maybe it would even make sense to move Queries to the new architecture of client adapters (https://github.com/alpheios-project/components/issues/264). Their purpose is to receive data from external sources, and that's what client adapters are for.

I like the notion of a QueryController, because that's what current implementation of LexicalQuery is. It makes several atomic requests (to morph analyzer service, to lemma translation, etc.) and then combine data from them, making some decisions that depend on if and what data has been returned.

So I think we could have two levels of objects in client adapters library: basic (atomic, or adapter-level) queries and more complex composite queries (being instances of QueryController probably). An atomic query would just send data to the remote service and get data back, without any business logic for data processing, except for parsing. A composite query would analyze data received from an atomic query, process it, if necessary, send other atomic queries based on results of data processing, and notify event subscribers (i.e. a UIController) when a new piece of data would become available. In this architecture a QueryController would to atomic queries be something that UIController is to Vue components.

What do you think of such approach in general?

balmas commented 5 years ago

that sounds promising. I kind of like the idea of moving the QueryController to the adapters library, particularly now that the adapters are combined. Need to think about it a little bit more.

kirlat commented 5 years ago

@balmas, @irina060981: I've added a small diagram depicting they way that, as I think, UI Controller and the Query could interact. Please take a look at it when you will have time (it is really tiny): https://github.com/alpheios-project/documentation/blob/master/development/app-architecture.md#interaction-with-the-query

In this schema, Query uses events to pass data to a UI Controller and thus is fully decoupled from it. If this approach is OK, we can go ahead with the change and decouple Query from UI controller. Then we'll be free to move it to client adapters library or leave it where it is. Query would be an independent entity from now on.

I think this step is important now as it will allow us to refactor UI controller more freely as almost nothing will depend on its internal structure. That will provide a much more freedom in what we can do.

irina060981 commented 5 years ago

I have some questions what would be the communication workflow? Do LexicalQuery will produce some unified events (upon some generic event pattern)? Or we will add them each time we will add a new application as a requestor?

May be we should produce only one event type - "update data" with some delta and any application could use it as delta or as a whole object?

balmas commented 5 years ago

I think the event-based workflow is good. I agree with @irina060981 that we should put some thought to how the events are named and whether these are all 1 type of event, distinguished by the data. I think we will probably want to move more to the event-based model, such as handling of updates to user preferences, so we might want to be careful about how many distinct event types we create.

kirlat commented 5 years ago

I'm not completely sure what the best way of implement this would be, but so far the idea is:

irina060981 commented 5 years ago

I think here it is not much difference really - we could have 2 different arguments (eventName, eventParam) and all those who will use them will have to know all those names or we could have the first agument the same - updateData and eventParam will have (eventName and eventParams)

I think it will have more flexibility - and we will be able to filter data more easier - what do you tnink, @kirlat ?

balmas commented 5 years ago

We need also to think about how errors are sent and handled. We have some improvements to do here (see alpheios-project/components#277).

kirlat commented 5 years ago

@irina060981, do I understand correctly that you suggest to have only one type of event listener callback (updateData) that will be called every time some information is retrieved from LexicalQuery? I agree that it's simpler to register one callback than several, but I'm not happy that in that case we have to do routing within a UI controller.

Let's compare two variants (let's say we have four events, A through D, and four methods to handle each, but we might have more):

  1. Universal update callback Registers with 

    LexicalQuery.addEvtListener(this.updateCallback)

    that's nice and short, but then we have to do the routing like:

    updateCallback (eventParam) {
if (eventParam.eventName === "eventA") { 
 this.updateA(eventParam.eventParams) 
} else if (eventParam.eventName === "eventB") {
this.updateB(eventParam.eventParams) 
} else (eventParam.eventName === "eventC") {
this.updateC(eventParam.eventParams) 
} else (eventParam.eventName === "eventD") {
this.updateD(eventParam.eventParams) 
}
}

    that's more tedious and repetitive, on my opinion, and is harder to read.

  2. Specialized callbacks Register's with (if we provide a group registration method)

    LexicalQuery.addEvtListeners({ eventA: this.updateA, eventB: this.updateB, eventC: this.updateC, eventD: this.updateD})

    and that's it, we don't need the second part.

I think specialized callbacks are more concise and more elegant. I also like the idea of removing routing code from the UI controller as it is, technically speaking, an auxiliary functionality to it. I would really love to have UI controller simpler. I think it's overburdened with convoluted logic now.

Are your concerns are that with specialized callbacks we have to know all event callback types and if we don't we would skip some data update? I see this as an advantage. If we skip an event we don't know (and don't care about too), that's fine.

It's good to isolate events that we know about (and know how to process its data) from the ones that are unknown to us. LexicalQuery is used by UIController only now, but it might be used by some other client (Games? something else?). For the second client a LexicalQuery may produce events and data formats that UIController knows nothing about (and don't need them). But those would still go into the UIController universal callback (as universal controller be called for any type of event and event data), even though UIController don't need this data and does not know how to process that. That will result in unnecessary callback calls and also, in theory, as some unknown data would sneak into a universal callback this may mess up our processing logic. I think it's better to avoid that. Might be very hard to debug (being there, done that 🙂). I think it's better to have isolated specialized callback functions. They are easier to mock and to test.

What do you think? Do I understand your reasons correctly?

In fact, we could combine two approaches, if we need, because we could register both specialized and universal callbacks at the same time, if we have a reason for that.

kirlat commented 5 years ago

@balmas, @irina060981: I've added a preliminary UI Controller architecture: https://github.com/alpheios-project/documentation/blob/master/development/app-architecture.md#ui-controller-architecture. Please let me know what do you think.

balmas commented 5 years ago

I think you make a good argument about the event handlers, @kirlat . Particularly this:

"For the second client a LexicalQuery may produce events and data formats that UIController knows nothing about (and don't need them). But those would still go into the UIController universal callback (as universal controller be called for any type of event and event data), even though UIController don't need this data and does not know how to process that."

The architecture diagram also looks fine to me. I'm still a little uneasy about increasing our dependency on Vue so if there is a way to abstract the Vuex store a bit, so that if we ended up swapping it out for something else we could minimize the changes I would be more comfortable, but I don't think it's worth a lot of extra work. I'd like just always to be careful when we introduce a new 3rd party dependency.

irina060981 commented 5 years ago

@kirlat , may be you are right. And I am not against it because it could be re-changed if it won't be efficient. Then we should remember about the following disadvantages of your approach:

1) We will create string unique identifiers to use as an event name - they should be uniqie in wide range of areas as we will use those events in various unknown sites that could have there own event handling code.

2) We should pay attention to any renaming it (as we could easily loose it)

3) We would create additional events for different situations with varios names or we will create the mini-generic events and would be analyze it (I think we could use more elegant way with dictionaries) with switch-case logic.

4) client-adapters repo with it LexicalQuery module would be not so clear and simple and would have additional dictionary/list of available events. Or it anyway becomes to be a little generic :) for example

It seems to me that we will to create switch-case parsing the event data anyway.

kirlat commented 5 years ago

@irina060981! Thanks for your comments, you're rising some very important points! They made me think about things I would've not think otherwise (especially on a design stage) and gave me some ideas to solutions that, I think, would benefit us.

Here is how, on my opinion, we could handle some of the situations you've described.

  1. We will create string unique identifiers to use as an event name - they should be uniqie in wide range of areas as we will use those events in various unknown sites that could have there own event handling code.

I think as we are not going to create a global event loops (when an event is visible to all objects within an application) that should be less of our concerns. We could use a subscription model, where a subscriber (i.e. a UI Controller) would subscribe to listen events of a particular object class only (i.e. a LexicalQuery) like LexicalQuery.addEvtListener("evntA", evtListener). With this, UI Controller would receive evntA notifications from LexicalQuery only. If, however, there is evntA fired within some other object, UI Controller does not care, because it is not subscribed to it. But if a UI Controller wants to listen to evntA from ResourceQuery, for example, it can subscribe to that too with ResourceQuery.addEvtListener("evntA", someOtherEvtListener). As a result, evtA of one class would be routed to one callback and of the other one to the other and there will be no collision.

Through a subscription model the names of the events are localized within classes that fires them. It's like class names being namespaces for that events: "evntA" fired by LexicalQuery would be separated from "evntA" fired by ResourceQuery because they will have different listeners. We just need to care so there there will be no two events with the same name within an originator class (i.e. LexicalQuery), but that's easy to do. In fact, the event handling implementation will prevent that (as event names would probably be used as map keys).

In fact, we can signify the naming convention described above by establishing a rule of adding originator name in front of an event name, so we could have LexicalQuery.evntA and ResourceQuery.evntA. Then by looking at the event name we could clearly understand where it came from. What do you think about that?

  1. We should pay attention to any renaming it (as we could easily loose it)

That's very important, but I think we can create some safety mechanisms to prevent that. Let's say we have evntA and a code that listens to it by subscribing as LexicalQuery.addEvt("evntA", listener). Then, if we decide to rename evntA to evntB inside a LexicalQuery, the listener() would never get called. That's really hard to catch!

But we can add a safety check to addEvt()so that if something tries to subscribe to evntA that does not exist any more (as it's being reamed), it will throw a warning or an error. As one more check we could, as you suggested, to add an event name as a prop to the callback arg data structure. Then, inside a callback, if we want to be very careful, we could check if (param.evntName === 'LexicalQuery.evntA) to make sure we're dealing with the right one. What do you think?

  1. We would create additional events for different situations with varios names or we will create the mini-generic events and would be analyze it (I think we could use more elegant way with dictionaries) with switch-case logic.

If it will be better to use mini generic events, we can use those, I think, They just should really try to stay on the "mini" and be specialized. Such code is better structured and easier to manage.

We don't need to force one way or the other. The callback structure would allow as to combine mini generic events with smaller specialized ones. The architecture is flexible enough to handle that.

  1. client-adapters repo with it LexicalQuery module would be not so clear and simple and would have additional dictionary/list of available events. Or it anyway becomes to be a little generic :) for example

I was thinking to create some generic event-handling classes as we did with a messenger service. Queries can then either inherit from them or include them with composition. So there would be no duplication of an event-handling code within each query class.

This can still be generic if we use generic dictionaries to handle routing. We can create a map were keys would be event names and values would be arrays of callback functions. So:

addEvtListener(evntName, callback) {
if (!this.handlers.has(evntName)) { throw new Error('This event is not supported') } // Safety check
this.handlers.get(eventName).push(callack) // Callback are stored in an array 
}

fireEvt(evntName, evntData) {
  let handlers = this.handlers.get(evntName)
  for (let handler of handlers) { handler(evntDat) }
}

// A discovery service, lists all available events, clients can check that
evntList() {
  return this.handlers.keys()
}

I think that's pretty compact and generic.

As for error handling I did not have time to think about that yet. The simplest solution would probably be to return a status code in the data object (one code for success, another one for data not found, yet another one for error, etc.) and check for that code within a callback. I understand the drawbacks of this approach (hard to maintain and sync list of codes between two parties, need a special knowledge of what each code mean), but we could probably mitigate that by providing methods for checking status of data within LexicalQuery (where those codes are generated). LexicalQuery.isNoData(evntData) would return true if there is a "NoDataFound" code in the event data and so on. Then a UI Controller don't need to know about error codes at all. Or maybe there are some better solutions to that? Do you have any ideas how to handle this better?

balmas commented 5 years ago

In fact, we can signify the naming convention described above by establishing a rule of adding originator name in front of an event name, so we could have LexicalQuery.evntA and ResourceQuery.evntA. Then by looking at the event name we could clearly understand where it came from. What do you think about that?

I think this is a good idea. I suggest maybe also that we use the Alpheios namespace for all Alpheios events.

balmas commented 5 years ago

As for error handling I did not have time to think about that yet. The simplest solution would probably be to return a status code in the data object (one code for success, another one for data not found, yet another one for error, etc.) and check for that code within a callback. I understand the drawbacks of this approach (hard to maintain and sync list of codes between two parties, need a special knowledge of what each code mean), but we could probably mitigate that by providing methods for checking status of data within LexicalQuery (where those codes are generated). LexicalQuery.isNoData(evntData) would return true if there is a "NoDataFound" code in the event data and so on. Then a UI Controller don't need to know about error codes at all. Or maybe there are some better solutions to that? Do you have any ideas how to handle this better?

This is a debate that programmers have been having since the beginning of time, but I think I would rather use Error classes and inheritance than error codes. I think we can probably identify a few meta classes of errors that can be used as needed, and further specialized when necessary by deriving from them, being sure not to duplicate unnecessarily the core javascript error types (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error) .

So, just off the top of my head we might have AlpheiosServiceTimeoutError, AlpheiosDataNotFound, AlpheiosServiceException and then the adapter classes can be responsible for translating service or request specific errors into these more general classes, wrapping the original error when necessary. How does that approach sound to both of you?

kirlat commented 5 years ago

I suggest maybe also that we use the Alpheios namespace for all Alpheios events.

Great suggestion. Then, I think, we could have it like Alpheios.LexicalQuery.EventA. Should be safe and self-explanatory.

I also like the idea of passing instances of Error into callbacks. We could integrate error reporting into an Error class and not worry about what error message should be shown by the UI Controller if an error occurred (and not to change it in several places if we decide to change it). If UI controller wants to report an error, it can just use a method of an Error instance to get that. We could integrate translations in there too.

But I think error codes could be important still for user reporting. It would probably easier for user to memorize and report: a number than an error name (or would it? What would be simpler: "58017" or "AlpheiosLexicalQueryTimeout"? I think probably the number is. People are used to memorizing or recording numbers more then they are to EXACT messages, I think). Based on the answer we might consider adding codes for the reporting purpose. Also, using codes in logs would spare us some space too.

The only question I have whether we would treat "DataNotFound" as an error. Technically, it is not an error, but a normal execution condition, so it would probably be wrong to treat it as an error. Maybe make an AlpheiosDataNotFound class to inherit from Object instead of Error ? What do you think?

irina060981 commented 5 years ago

@kirlat , It seems to me that you were talking about to use some custom event's solution (as described in the stackoverflow example). I think we should use some more tested solution for getting eventEmitters that could be used in different browsers and nodejs - for example this one - https://github.com/primus/eventemitter3 It has enough stars I think :)

About using the events - If I understood correctly you suggest to use it this way:

import the LexicalQuery module attach some eventEmitter to it and store in some global scope - otherwise (if I am right of course) it will be deleted So it could increase the used RAM.

I am not sure here if to be honestly, because I used before some already created in global scope for listening to events (window, browser, node events). What do you think?

kirlat commented 5 years ago

@irina060981: Yes, I was thinking about a custom solution. The reason for this is that custom solution is so simple, we'll create it faster than we would integrate some more complex (and powerful) solutions.

You're right that we have to keep data in global scope (in fact, it is module scope, so we're safe from collisions). However, a simple event emitter will not take much memory at all. All it takes is to have a map that will store event names and callback references. That's nothing comparing with all the lexical data we have to keep. Events in the solution I was thinking about are kind of global, but I think it would make sense for us to localize them.

I created a working concept of what I had in mind (because talk is cheap 🙂). We can test it, evaluate, and decide whether it'll be a good on for us. The whole thing is less than 50 lines long: https://github.com/alpheios-project/components/blob/ui-controller-refactoring/src/lib/events/event-emitter.mjs

To test it, check out a ui-controller-refactoring branch of components and run test.mjs from src/lib/events as node --experimental-modules test.mjs. It creates a few queries that emit events. All functionality we need is already in place.

It works in any environment because it's not environment dependent at all.

Solution that you provided a link for looks great and very iteresting, and there are some other good solutions too. I agree that it's better not to reinvent the wheel. However, when solution is as simple as above, I'm wondering if we really need anything more complex than that.

Comparing third-party vs in-house solutions, I think the following is true. Please add your thought and correct me if I'm wrong.

Third-party pros: very powerful and feature rich (but probably we won't use most of them), well optimized code, well tested, constantly (hopefully) developed so we might expect some new features

Third-party cons: one more external dependency. This means increased build size because it has functionality we'll probably not use. If project be abandoned, we're in trouble. It's also harder and slower to fix issues as we have to submit them to upstream for that.

From my experience, if we need something really simple, as a couple of utility functions, we better write and maintain them ourselves. If we need something that take us a lot of time to write (as interact.js or even Vue.js), we better use third party solutions. We just need to find the right balance here.

Node.js has its own global events. Browsers has global DOM events too. My idea was that we don't need anything as global as that, but rather a simple solution to send events between couple of objects.

However, if we would decide on complex event based architecture where all components would send events to each other, then probably we need something more complex than that. I think ideally we should use something that fulfill our needs and won't burden us with functionality we don't care about. So the main question, probably, is what do we need from an event emitter? Then it will be much simpler to find a solution to satisfy our requirements. @balmas, what do you think about that?

irina060981 commented 5 years ago

@kirlat , if you don't think that there is a need in an existed solution you could use your own of course. And if there exists such need - it will become clear. From my experience such solutions are often better - because they are tested better, they may be tested in different environments with the help of users of such packages. But I think that such choices are personal :)

balmas commented 5 years ago

I think all uses of 3rd party libraries need careful consideration. It's always a balance between not reinventing the wheel and becoming too dependent on code that isn't fully under our control and which might itself introduce long chains of dependencies.

That said, I did take a quick look at eventemitter3 and it looks like a quite clean, well-commented piece of code that doesn't introduce any other dependencies. However it is also written in EMCA script 3 not 6, and so that is something to consider too, whether at some point that might be problematic.

We have a lot to do and I don't want to get too bogged down in this choice. @kirlat your solution also looks like it might be enough to get started on this, but I would want to see unit tests added for it from the start. I do agree with @irina060981 that a well-tested solution has value.

kirlat commented 5 years ago

Splited discussion of communication protocol between components into a separate issue: https://github.com/alpheios-project/documentation/issues/6

balmas commented 4 years ago

this has largely been implemented.