refactor(ContextLoader): remove ClassLoader
ClassLoader does nothing but create itself. So why bother adding a new concept?
Checklist
- [x]
npm testpasses - [x] commit message follows commit guidelines
Affected core subsystem(s)
ContextLoader
Description of change
-
remove ClassLoader. Instead create a plain object named
namespaceingetIntance() -
use
definePropertyGetter()to define getters for all service instances (e.g.bofservice.a.b), serivce namespaces (e.g.aofservice.a.b) andctx.serviceitself
Codecov Report
Merging #185 into master will not change coverage. The diff coverage is
100%.
@@ Coverage Diff @@
## master #185 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 19 19
Lines 1068 1055 -13
=====================================
- Hits 1068 1055 -13
| Impacted Files | Coverage Δ | |
|---|---|---|
| lib/loader/context_loader.js | 100% <100%> (ø) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 136ad7b...b944a4f. Read the comment docs.
Will it improve performance? In my opinion, Class is more readable than function, so I don't think it need be changed.
Well, for me a new concept named ClassLoader is more confusing. When i just see the name without reading the details i guess it is similar to ContextLoader or FileLoader, and it should have a load() method. And a 'loader' is expected to work during boot, but the fact is that the ClassLoader is instantiated for a special ctx.
The ClassLoader has no behavior but creating an object with some getters. In other words, it just provides a namespace. i think there is no need to declare a special ClassLoader.
In addition, a getter named serivce on ctx and a getter named foo on ctx.service are similar inside. Both they lazily initialize a instance and cache it with a Map. So i could naturally use definePropertyGetter() to handle these.
Thanks for more review and consideration.