toplan / laravel-sms

:iphone::heavy_check_mark:A phone number validation solution based on laravel
MIT License
839 stars 174 forks source link

一个关于类名规范的建议 #82

Closed mogody closed 8 years ago

mogody commented 8 years ago

建议把底层的类名改为PhpSms,和对外提供的一样。不一样的话在触发事件的时候,由于对外提供的是PhpSms,很容易让人误解:

$sms = PhpSms::make()->to($request->input('phone'))->template([
            'Alidayu' => '123'
        ])->data([
            'code' => (string)$verfiyCode,
            'product' => '123'
        ]);

        dd( event(new SendSms($sms)) );

如下这段代码,在触发事件的时候需要一个短信对象,这个对象类型必须用Sms提示,但是对外提供的却是PhpSms,如果没去看源码,肯定会掉坑里面

mogody commented 8 years ago

还有,在使用阿里大鱼的服务商时候,建议把模版数据类型在底层做一个转换,因为验证码一般是随机函数生成,是一个整型类型的,而阿里大鱼对于模版数据必须是字符型,这样如果用户忘记做转换就会出现JSON数据类型异常

mogody commented 8 years ago

阿里大鱼代理器在底层出现了一个数组越界异常,虽然短信可以发送成功,但希望可以修复。 qq 20160517095343 原因是阿里大鱼的响应数据改了,但是文档却没有写,所以掉到了这个坑。 看样子你这项目应该是没有做测试吧

toplan commented 8 years ago

ok,你说的还是很有道理,这个issue适合于phpsms。phpsms大部分做了测试的,但是对于代理器类,因为感觉做网络测试,时间很长,就放弃了,你有什么好的建议吗?laravel-sms没有做测试。

mogody commented 8 years ago

@toplan 恩确实,我的想法是把代理器接口化,方便拓展。然后返回值可以参考Laravel的Collection,统一一下,因为各个服务商的响应数据各有千秋,统一了也容易使用。这样子在测试的时候我觉得会方便一些

toplan commented 8 years ago

现在所有的代理器都是继承Toplan\PhpSms\Agent抽象类,里面封装了一起基础方法,并且会把所有代理器的返回值统一组装为以下结构:

[
'success' => true/false,
'code' => ...
'info' => ..
]

基本算是接口化了并统一了响应数据,不过我在phpsms v2版本的规划大概是这样:

Sms::to()->content()->template()->data()->send(); //短信全能类

TemplateSms::to()->template()->data()->send(); // TemplateSms非全能,没有content方法
Dispacher::scheme(...);
Dispacher::config(...);
Dispacher::queue(...);
...
toplan commented 8 years ago

@wh469012917 dev-master已更新,解决阿里大鱼的问题,希望你能体验下 😄

mogody commented 8 years ago

嗯,客气了,很不错的项目。 你可以参考下策略模式,建造者模式,可能会对开发有一些帮助

toplan commented 8 years ago

感谢支持。已发布2.4.0