20
回答
PHP 这种编程方式好吗?
利用AWS快速构建适用于生产的无服务器应用程序,免费试用12个月>>>   

我是一个建站爱好者,学了一段时间的PHP,最近在开发一个网站。

发现程序中的各种条件判断让我头晕,于是有了下面这段代码,大家觉得这种款式的代码执行效率如何。

之间的代码确实太吓人,现在的会不会好点。

    private function message($error, $message='', $url='')
    {
        header('Content-type: text/html; charset=UTF-8');
        $msg = array(
            'error'  => $error,
            'message' => $message,
            'url' => $url
        );
        echo json_encode($msg);
        exit();
    }
    
    public function uploadImgAction()
    {        
        //校验用户权限
        if ($this->user->checkLogonStatus() == false)
            $this->message(1, '只允许登录用户上传图片');
        
        $allow = array('gif', 'png', 'jpg', 'jpeg');
        $maxsize = 1024 * 1024;     // 1MB
        
        //设置附件的频道
        $channel = isset($this->params[0]) ? trim($this->params[0]) : '';
        if (!in_array($channel, $this->channel))
            $this->message(1, '末定义的频道');;  
            
        if (empty($_FILES['imgFile']['name']))
            $this->message(1, '请选择要上传的图片');
        
        if ($_FILES['imgFile']['size'] > $maxsize)
            $this->message(1, '图片大小超过限制');
            
        $arr = explode('.', $_FILES['imgFile']['name']);
        $ext = array_pop($arr);
        $ext = strtolower(trim($ext));
        
        if (in_array($ext, $allow) == false)
            $this->message(1, '只允许上传图片文件');

        //设置归档路径
        $ymd = date('Y') . '/' . ceil(date('z', time()) / $this->ymd);
        //设置保存路径
        $savepath = $this->attachedPath . $channel . '/' . $ymd . '/';
        //设置保存路径URL
        $saveurl = $this->attachedUrl . $channel . '/' . $ymd . '/';
            
        if (!file_exists($savepath))
            pcb_makeDir($savepath);
            
        //设置文件名    
        $filename = $this->user->getID() . '-' . date('YmdHis') . '-' . mt_rand(11, 99) . '.' . $ext;
        //设置完整的文件保存路径
        $filepath = $savepath . $filename;
            
        //检测用户积分,没有积分禁止上传
        $userScore = $this->mysql->select('SELECT `score` FROM `pcb_user` WHERE `id` = ' . $this->user->getID() . ' LIMIT 1', false, 'score');
        if ($userScore <= 0)
            $this->message(1, '您的积分不足');
        
        //如果移动临时文件失败,返回提示信息
        if (move_uploaded_file($_FILES['imgFile']['tmp_name'], $filepath) == false)
            $this->message(1, '移动临时文件错误');
        
        //上传成功保存进数据库
        $path = str_replace(SYS_PATH, '/', $filepath);
        $this->insertDB($path);
            
        //扣除相应积分
        $score = isset($GLOBALS['score']['uploadImage']) ? intval($GLOBALS['score']['uploadImage']) : -2;
        $this->user->operaScore($this->user->getID(), $score);
            
        //设置图片URL
        $url = $saveurl . $filename;
        
        //返回JSON数据
        $this->message(0, '', $url);
    }

 

PHP
举报
AOWANA
发帖于5年前 20回/1K+阅
共有20个答案 最后回答: 5年前

提几点建议:

1. $allow 和 $maxsize 可以放在action外部定义

2. 创建路径(目录) 单独写一个方法,返回路径值

3. 图片上传 和 图片限制检测 单独写一个方法

4. 应当在权限和频道检测完成后 检测积分

5. 扣除积分 和 保存图片 应当为 事务

你好,你这个程序,如果有个否定的话,为什么不直接就跳出程序呢?你现在这么一写,第一步error=1了,还要跑后面一堆的判断....我依然都错了,那直接就跳出弹错误信息不就得了......你的错误信息还不是累记的....

其实...是不是可以考虑下try....catch....或者会更好!

--- 共有 1 条评论 ---
AOWANA晕,我只想着使用 exit() 结束,忘记可以再写一个函数专门返回JSON数据然后结束,多谢哥们啦。 5年前 回复

在我的代码里面不可能有这么长的函数,也不会有这么多if else

面向对象最重要的就是要用设计模式消除掉if else,switch这样的过程式的玩意。

--- 共有 2 条评论 ---
回去干活回复 @老朽 : 继续改,这个地方明显可以用通用的检测类库来处理,看看这里:https://github.com/laravel/laravel/blob/master/laravel/validator.php 5年前 回复
AOWANA确实太长了,头晕,现在改好了 5年前 回复

虽然代码很容易看懂,但确实很丑,这个if分支真是失败。

这样调试的时候是很晕的,也容易出bug(主要是重复或漏判断)

顶部