You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I would appreciate some feedback about the ideas I mention below and some of the solutions the commits tied to this ticket propose.
The App/Controller Idea
Originally the MapView class was called something like DncApp and it's general purpose was to glue and setup things:
setup and bind components being used in DNC, like the map
setup component event listeners, like for FileReader event handlers
it was a place for slim applciation logic, but not component specific stuff
When we decided that MapView felt like it was it's own component, things got shuffled around and some glue stuff didn't make sense to have in that class anymore.
I've noticed that some of the glue logic was moved out into L.DNC where it currently lives. My guess is that we can expect that glue to grow in the future. L.DNC really feels like a simple namespace object for things to hang off, but not it's own class. Maybe that's just me.
More importantly, another side effect of the refactor was the propagation of some patterns that I feel like we should avoid:
FileReader now reaches outside of it's scope globally to access other components here.
The TurfOperation also reaches outside of it's scope to add layers here.
Even more confusing, the FileReader component fires an event that it itself listens to here.
Proposal
It feels like we need a thing that does the glueing and setup again. This thing, lets call it a controller, would also hold component references so we don't force components that shouldn't know about each other to tightly reference each other.
What Changed?
I managed to remove all these type of references except one, which I flagged as a possible removal later.
For lack of a better term ( i'm open to anything ), I called the controller AppController. See the class here.
The text was updated successfully, but these errors were encountered:
Interesting thoughts, will look more into the proposals soon. Out of curiosity, why is this an Issue? Maybe we should just close this and move it all into the Pull Request? I typically think of Issues as TODOs.
I would appreciate some feedback about the ideas I mention below and some of the solutions the commits tied to this ticket propose.
The App/Controller Idea
Originally the
MapView
class was called something likeDncApp
and it's general purpose was to glue and setup things:FileReader
event handlersWhen we decided that
MapView
felt like it was it's own component, things got shuffled around and some glue stuff didn't make sense to have in that class anymore.I've noticed that some of the glue logic was moved out into
L.DNC
where it currently lives. My guess is that we can expect thatglue
to grow in the future.L.DNC
really feels like a simple namespace object for things to hang off, but not it's own class. Maybe that's just me.More importantly, another side effect of the refactor was the propagation of some patterns that I feel like we should avoid:
FileReader
now reaches outside of it's scope globally to access other components here.TurfOperation
also reaches outside of it's scope to add layers here.FileReader
component fires an event that it itself listens to here.Proposal
It feels like we need a thing that does the glueing and setup again. This thing, lets call it a controller, would also hold component references so we don't force components that shouldn't know about each other to tightly reference each other.
What Changed?
AppController
. See the class here.The text was updated successfully, but these errors were encountered: