li6185377 / LKDBHelper-SQLite-ORM

全自动的插入,查询,更新,删除, an automatic database operation thread-safe and not afraid of recursive deadlock
MIT License
1.21k stars 288 forks source link

关于 +getModelInfos 方法的逻辑问题 #138

Closed wving5 closed 5 years ago

wving5 commented 5 years ago

Hi, 尊敬的作者! 首先非常感谢你的开源 👍 ~ 该项目对我很有帮助

最近在项目中实践时,思考如何在多级继承的情况下,更好地使用 + getTableMapping 方法,并在翻阅相关代码的途中遇到了疑惑,想跟你探讨一下。如有表述不当的地方请尽管指出 😃


+ getModelInfos 这个方法中的下面 2 段代码, 个人认为在逻辑上有部分重复

第一段代码

https://github.com/li6185377/LKDBHelper-SQLite-ORM/blob/246d107b78d68dfcebdf6ad457470993fb6b60a9/LKDBHelper/Helper/NSObject%2BLKModel.m#L737-L739

+ getSelfPropertys: protypes: 这个方法的末尾有根据 isContainParent 递归调用 superClass 的实现。 https://github.com/li6185377/LKDBHelper-SQLite-ORM/blob/246d107b78d68dfcebdf6ad457470993fb6b60a9/LKDBHelper/Helper/NSObject%2BLKModel.m#L873-L875

所以上面的代码执行完,已经拿到了自身及所有父类的全部 property 。

我们将它记作 iVarList_Full_Origin

PS: 翻了下 commit 记录这一段在 3d2d84b 曾删掉, e06d42f 又加回来了, 但是没看明白原因……不知是否有助于你回忆

第二段代码

https://github.com/li6185377/LKDBHelper-SQLite-ORM/blob/246d107b78d68dfcebdf6ad457470993fb6b60a9/LKDBHelper/Helper/NSObject%2BLKModel.m#L749-L758

这里根据isContainParent 递归调用了 superClass 的 + getModelInfos

与上面的区别在 + getModelInfos 会调用 + getTableMapping。 而 + getTableMapping 有个效果,直接决定只保留哪些 property


- (id)initWithKeyMapping:(NSDictionary *)keyMapping propertyNames:(NSArray *)propertyNames propertyType:(NSArray *)propertyType primaryKeys:(NSArray *)primaryKeys
{
......
// 这里判断
if (keyMapping.count > 0) {
NSArray *sql_names = keyMapping.allKeys;
for (NSInteger i = 0; i < sql_names.count; i++) {
......
            [self addDBPropertyWithType:type cname:column_name ctype:column_type pname:property_name ptype:property_type];
        }
    } else {
        for (NSInteger i = 0; i < propertyNames.count; i++) {
        ......

            [self addDBPropertyWithType:type cname:column_name ctype:column_type pname:property_name ptype:property_type];
        }
    }


> 所以这段代码得到的是, superClass 的 **iVarList_Full_Origin** 经过 `+ getTableMapping` 过滤(也可能未被过滤)的 property 集合。 

我们权且将它记作 **iVarList_Full_Mapping**   ( 必然是  **iVarList_Full_Origin**  的子集 )

### #理一下 `+ getModelInfos` 
1. 获取**当前 class** `keyMapping`
https://github.com/li6185377/LKDBHelper-SQLite-ORM/blob/246d107b78d68dfcebdf6ad457470993fb6b60a9/LKDBHelper/Helper/NSObject%2BLKModel.m#L735
2. 拿到  **iVarList_Full_Origin** 放入 `pronames` & `protypes`
3. 拿到 **superClass** 的 **iVarList_Full_Mapping** ,也放入 `pronames` & `protypes`
3.1. 这里会递归父类 `+ getModelInfos` ,先不用关心
4. 一起作为入参传入 `initWithKeyMapping: propertyNames: propertyType:`, 得到最终的 `infos` (也即**当前 class** 的 **iVarList_Full_Mapping**)
4.1 这里会发生前面提到的,当前 class 的 `keyMapping` 直接决定保留哪些字段(包括父类的)

### # 两个疑惑
1. 关于`+ getModelInfos` 逻辑。因为 `父类的 **iVarList_Full_Mapping** < 父类的 **iVarList_Full_Origin** < 子类的 **iVarList_Full_Origin** ` ,所以上述 步骤3 便**没有存在的意义**了。 
1.1 继承关系有 N 层的情况下( `isContainParent` 都为 YES ),步骤 2 的递归函数 `+ getSelfPropertys: protypes:`  会被调用 `(N+1)*N/2` 次

2. 关于`+ getTableMapping` 最佳实践。已知它有几个作用,一是取舍冗余的字段,一是映射sqlite列名(混淆/预留都很方便),一是自定义值的转换。 这些设计得非常好用。可是**一旦父类实现**该方法:
2.1  子类不重写的话,会直接调到父类,子类自己的属性就没了 😢  于是必须重写
2.2  重写并偷懒直接返回 `nil`,则父类规定的 mapping 就没了 😢 
2.3  重写并抄写子类的属性,再合并 `[super getTableMapping]` 。完美 🌚 。但如果父类是个使用广泛的基类,大概需要脚本来生成…  样板代码就多了,而且不是很便利
2.4  或者父类尽量不实现该方法,由子类实现并直接 hardcode 父类的字段。也不优雅,且同样有样板代码多的问题...

### #跑题
#### 纠结的 tableMapping 临时方案
虽然子类重写 `+ getTableMapping` 本是天经地义。但抱着能不能把懒偷了,也达到差不多的效果的想法...…  基于目前项目需求,我对 `+ getTableMapping` 的诉求只有**去除冗余字段**, 以及子类默认继承父类的 mapping,只需定义自己的属性 mapping。 

据此简单修改了  `+ getSelfPropertys: protypes:` 只返回自身(不递归父类) 属性,同时需要子类默认重写`+ getTableMapping`并返回 `nil`。勉强达到了上述目的。

但是这样做不能自动继承 LKSQL_Mapping_Binding / LKSQL_Mapping_UserCalculate 的特性, 担心以后会心塞。 回头再试试把父类的 `LKDBProperty` copy 一份添加给子类怎么样。

#### 总结
出现这样的矛盾,本质上是因为最终整合的 `+ getTableMapping` 的信息跟**获取 property 列表** 没有"对应"上。(现在是递归出所有自身及所有父类属性的 name & type ,再用**当前 class ** `+ getTableMapping` 一刀切) 等于硬性要求子类必须继承 `+ getTableMapping` 并调用 `super`

个人想法是否可以每个 class 只管理自己的属性 mapping。 
假设有 A, B:A, C:B, D:C ,则有
ALL mapping for Class D: [A.mapping] - [B.mapping] - [C.mapping] - [D.mapping] 
ALL property for Class D: [A.property] - [B.property] - [C.property] - [D.property] 
递归父类 D->C->B->A 逐级得到 `LKModelInfos` 然后合并得到 D 的完整 ModelInfos

 不知阁下意见如何,望回复 :)
wving5 commented 5 years ago

可以 review 下 7ed48ba0a38ba9f74642535bec6a167ba612fde7 8311c03920c8260c4af554116e1820298d2d0e40 881275430879563a35d50fdf8ef90a3a46b34a89

li6185377 commented 5 years ago

天啦,牛逼。。。 改动的代码太多了,我自己也忘了当初为啥那样写了。。。 只记得有个原因 NSMutableArray, NSMutableDictionary 等可变对象 ,读取的时候 会保证生成的 Class 也是mutable 的 。。 其他都忘了,可能是为了偷懒,就把 mapping 和 property 合在一起了

wving5 commented 5 years ago

只记得有个原因 NSMutableArray, NSMutableDictionary 等可变对象 ,读取的时候 会保证生成的 Class 也是mutable 的 。。

可能真的太久了 😂 这个 mutable 的问题以前有人提过 issue 你 fix 掉了,在这个提交 199a234 ,跟 json 反序列化有关系。应该跟 getModelInfos 这边的逻辑没什么关系哦。。

我也加了个 test 验证确实不相关 f9cdc45da60cf79a4ff754675bfbd93c14802497

li6185377 commented 5 years ago

感谢你这么仔细的阅读这个项目,如果有改进的地方 可以直接提 pull request 进来

wving5 commented 5 years ago

客气了~ 其实应该感谢你还在社区积极维护才对~ 好榜样 👍 关于PR,回头看自己的改动更偏向个人需求,貌似不太适合 PR 😂

改进

其他 (可以无视