Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

关于lib的几个问题 #1

Closed
otakustay opened this issue Mar 5, 2013 · 9 comments
Closed

关于lib的几个问题 #1

otakustay opened this issue Mar 5, 2013 · 9 comments
Labels

Comments

@otakustay
Copy link
Member

看了下lib的函数,总结一下几个问题:

保持函数的简单性

比如bind函数,原实现是可以从scope中取fun为名字的方法的,我认为这是原tangram为了某些业务线的需求而产生的功能,我们不应该鼓励和提倡这种用法,因此我去掉了。

同样的问题在很多函数中都存在,例如:

  • format函数支持${name}{index}两种形式,是否只支持${name}就OK了
  • hasClassaddClassremoveClass支持多个class以空格分隔,确实这功能非常强大,jQuery也提供这一功能,但在一个UI控件体系中,代码都是可掌握的高质量代码,是否有必要提供这种 看似便利实则可能导致混乱和不一致性 的功能

关于继承

tangram的继承实现是没错的,但是它提供了一些额外的东西,包括:

  • 一个__type属性表示类型
  • 一个superClass属性引用基类
  • 一个extends方法生产子类

其中superClass比较广泛,可以认可(虽然我个人很反对这一做法),但是__typeextends从我的角度来看是完全没有存在的意义的,是否还需要保留。

关于深层次结构嵌套

现有的lib其实是比较混乱的,语言层面的东西(trimclone等)、字符串层面的东西(encodeHTML等)和DOM层面的东西(ghasClass等)混在一个下面,这种设计是没有问题的,我个人也很推崇这种,毕竟没多少东西,不必要再细分。

但是奇怪的是,有一个page二级“命名空间”,这货的存在意义是什么……?

题外话:如果现在lib中的东西是原封不动从tangram弄过来的话,这大概是我第一次认真地看tangram的源码,只想吐槽这代码质量是有多差……

@musicode
Copy link

musicode commented Mar 6, 2013

addClass中关于多个class以空格分割的问题,我觉得是为性能考虑吧,毕竟多次调用 addClass 看起来也不爽,折衷一下,可以实现为 addClass(class1, class2, ...),这样也清晰了

@otakustay
Copy link
Member Author

addClassremoveClass接受多个class是可以理解的,问题就是应用的场景有多少(从控件的抽象体系来看)

但是hasClass多个class似乎并不是一个很好的设计,其接口的理解性很低,多个class之间是OR还是AND的关系?

@leeight
Copy link
Member

leeight commented Mar 6, 2013

题外话:如果现在lib中的东西是原封不动从tangram弄过来的话,这大概是我第一次认真地看tangram的源码,只想吐槽这代码质量是有多差……

历史问题,这些代码都是好几年前的了,可以理解的。

有些实现尽可能采用浏览器的实现,比如addClass,hasClass,bind之类的?

@errorrik
Copy link

errorrik commented Mar 6, 2013

我觉得尽量简单点,我们是为ui库做lib抽象,不是为业务开发做lib。尽量采用浏览器实现我觉得也是OK的(如classList)

@errorrik
Copy link

errorrik commented Mar 6, 2013

还有我稍微解释下,正则用new RegExp(\x24)不用/...$/原因是ctpl发现$就会去做变量替换,所以tangram中基本所有正则都是new

@otakustay
Copy link
Member Author

@errorrik 所以我可以修改一下这里的实现,用正则直接量是吧?

@leeight @errorrik 关于使用浏览器原生实现,我思考这个问题没有1年也有个9月了,我的疑惑是,既然还是需要兼容性的实现代码,那么增加一个if分支,额外增加脚本体积,换过来的是什么,一些的性能提升能够成为解决瓶颈的关键吗?(当然显得很牛逼这种理由就不需要了)

@otakustay
Copy link
Member Author

补充一下,bind这个函数我会倾向于尽量用原生的实现,是因为原生的bind其实是通过[[TargetFunction]]搞的,一个好处是,单步调试的时候,一次step in就可以跳到原函数去,而不原生实现至少要过一个闭包,step in 按到手酸。而一般调试都是在支持bind的浏览器下玩的……

@errorrik
Copy link

errorrik commented Mar 7, 2013

@otakustay
嗯,是的,可以用正则直接量。我自己也很烦new RegExp,性能又差又难看

@otakustay
Copy link
Member Author

d3354b8 做了下代码的清理:

  • 正则用直接量
  • format只支持${name}形式的
  • hasClassaddClassremoveClass只支持一个className
  • 添加toggleClass
  • inherits去掉__typeextends这几个东西的逻辑
  • bind接口与Function.prototype.bind一致
  • 保留pagedate二级命名空间

otakustay pushed a commit that referenced this issue May 7, 2013
bobshen added a commit that referenced this issue Jul 31, 2013
Fixed Issue  #129
Enhanced Problem #1 of Issue #105
otakustay pushed a commit that referenced this issue Jul 10, 2014
yankun01 added a commit that referenced this issue Apr 16, 2015
strwind added a commit that referenced this issue Apr 23, 2015
yankun01 pushed a commit that referenced this issue Jul 20, 2015
yankun01 pushed a commit that referenced this issue Aug 27, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants