php - 重构技巧

Question

我真的没有保理方面的经验。我的代码很长，我不使用函数，因为我不知道它是否需要成为函数。我希望你能给我一些提示，以便我可以清理我的代码。

<?php

# Required files
include("simple-html-dom.php");
require("{$_SERVER['DOCUMENT_ROOT']}/config/pipeline-x.php");

# Define variables
$fn = urlencode($_REQUEST['fn']);
$ln = urlencode($_REQUEST['ln']);

# Connect to database
$db = new px_dbasei();
$db->connect("192.168.50.70", "****", "****", "piasdgeline_tesh45t");

# Query database if a record exist
$sql = "SELECT * FROM linkedin_parse "
       ."WHERE "
       ."`first_name` = '{$fn}' AND "
       ."`last_name` = '{$ln}' ";
$results = $db->query($sql);

# If there is no result
if($results->num_rows == 0):

    # Search linkedin and download page
    $ch = curl_init();
    curl_setopt($ch, CURLOPT_URL, "http://www.linkedin.com/pub/dir/?first={$fn}&last={$ln}&search=Search");
    curl_setopt($ch, CURLOPT_USERAGENT, "Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/20100101 Firefox/19.0");
    curl_setopt($ch, CURLOPT_HEADER, 0);
    curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
    curl_setopt($ch, CURLOPT_ENCODING, 'gzip,deflate');
    curl_setopt($ch, CURLOPT_TIMEOUT, 8);
    $res = curl_exec($ch);
    curl_close($ch);
    $html = str_get_html($res);

    # Parse records from the download page
    foreach($html->find('li.vcard') as $vcard):
            $table = array();  

        foreach($vcard->find('span.given-name') as $given_name):                    
            $table['first_name'] = (trim(addslashes($given_name->plaintext), " "));
        endforeach; 
        foreach($vcard->find('span.family-name') as $family_name):
            $table['last_name'] = (trim(addslashes($family_name->plaintext)," "));
        endforeach;
        foreach($vcard->find('span.location') as $location):
            $table['location'] = (trim(addslashes($location->plaintext), " "));
        endforeach;
        foreach($vcard->find('span.industry') as $industry):
            $table['industry'] = (trim(addslashes($industry->plaintext), " "));
        endforeach;
        foreach($vcard->find('dd.current-content') as $headline):
            $table['headline'] = (trim(addslashes($headline->plaintext), " "));
        endforeach;
        foreach($vcard->find('a.btn-primary') as $url):
            $table['url'] = addslashes($url->href);
        endforeach;

        # Insert generated results to the database
        $sql = "INSERT INTO linkedin_parse (`first_name`,`last_name`,`location`,`industry`,`headline`,`url`) "
              ."VALUES "
              ."('{$table['first_name']}',"
              ."'{$table['last_name']}',"
              ."'{$table['location']}',"
              ."'{$table['industry']}',"
              ."'{$table['headline']}',"
              ."'{$table['url']}')";
        $db->query($sql);

        # Get last insert id and query database again
        $new_id = $db->insert_id();
        $sql2 = "SELECT * FROM linkedin_parse WHERE `linkedin_parse_id` = '{$new_id}'";
        $result = $db->query($sql2);

        # Display results in HTML
        ?>
        <ol>
            <?php while($row = $result->fetch_assoc()): ?>
                <li class="vcard">
                    <span class="given-name"><?php echo $row['first_name'] ?></span>
                    <span class="family-name"><?php echo $row['last_name'] ?></span>
                    <span class="location"><?php echo $row['location'] ?></span>
                    <span class="industry"><?php echo $row['industry'] ?></span>
                    <dd class="current-content">
                        <span><?php echo $row['headline'] ?></span>
                    </dd>
                    <a href="<?php echo $row['url'] ?>"></a>
                </li>
            <?php endwhile; ?>
        </ol>
        <?php
    endforeach;
else:
    # Query database if record is 30 days old
    $sql = "SELECT * FROM linkedin_parse "
          ."WHERE "
          ."`first_name` = '{$fn}' AND"
          ."`last_name` = '{$ln}' AND"
          ."`date_inserted` >= DATE_SUB(NOW(), INTERVAL 30 DAY)";
    $results = $db->query($sql);

    if($results->num_rows != 0):
        # Retrieve from database
        $sql = "SELECT * FROM linkedin_parse "
          ."WHERE "
          ."`first_name` = '{$fn}' AND"
          ."`last_name` = '{$ln}' ";
        $result = $db->query($sql);

        # Display results in HTML
        ?>
        <ol>
            <?php while($row = $result->fetch_assoc()): ?>
                <li class="vcard">
                    <span class="given-name"><?php echo $row['first_name'] ?></span>
                    <span class="family-name"><?php echo $row['last_name'] ?></span>
                    <span class="location"><?php echo $row['location'] ?></span>
                    <span class="industry"><?php echo $row['industry'] ?></span>
                    <dd class="current-content">
                        <span><?php echo $row['headline'] ?></span>
                    </dd>
                    <a href="<?php echo $row['url'] ?>"></a>
                </li>
            <?php endwhile; ?>
        </ol>
        <?php
    else:
        # Search linked-in for updated records
            $ch = curl_init();
            curl_setopt($ch, CURLOPT_URL, "http://www.linkedin.com/pub/dir/?first={$fn}&last={$ln}&search=Search");
            curl_setopt($ch, CURLOPT_USERAGENT, "Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/20100101 Firefox/19.0");
            curl_setopt($ch, CURLOPT_HEADER, 0);
            curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
            curl_setopt($ch, CURLOPT_ENCODING, 'gzip,deflate');
            curl_setopt($ch, CURLOPT_TIMEOUT, 8);
            $res = curl_exec($ch);
            curl_close($ch);
            $html = str_get_html($res);

            # Parse records from the download page
            foreach($html->find('li.vcard') as $vcard):
                $table = array();  

                foreach($vcard->find('span.given-name') as $given_name):                    
                    $table['first_name'] = (trim(addslashes($given_name->plaintext), " "));
                endforeach; 
                foreach($vcard->find('span.family-name') as $family_name):
                    $table['last_name'] = (trim(addslashes($family_name->plaintext)," "));
                endforeach;
                foreach($vcard->find('span.location') as $location):
                    $table['location'] = (trim(addslashes($location->plaintext), " "));
                endforeach;
                foreach($vcard->find('span.industry') as $industry):
                    $table['industry'] = (trim(addslashes($industry->plaintext), " "));
                endforeach;
                foreach($vcard->find('dd.current-content') as $headline):
                    $table['headline'] = (trim(addslashes($headline->plaintext), " "));
                endforeach;
                foreach($vcard->find('a.btn-primary') as $url):
                    $table['url'] = addslashes($url->href);
                endforeach;

                # Update records
                $sql = "UPDATE linkedin_parse "
                      ."SET "
                      ."`date_inserted` = now(),"
                      ."`first_name` = '{$table['first_name']}',"
                      ."`last_name` = '{$table['last_name']}', "
                      ."`location` = '{$table['location']}', "
                      ."`industry` = '{$table['industry']}', "
                      ."`headline` = '{$table['headline']}', "
                      ."`url` = '{$table['url']}' "
                      ."WHERE "
                      ."`first_name` = '{$table['first_name']}' AND"
                      ."`last_name` = '{$table['last_name']}' AND "
                      ."`location` = '{$table['location']}' ";
                $result = $db->query($sql);
                ?>
                <ol>
                    <?php while($row = $result->fetch_assoc()): ?>
                        <li class="vcard">
                            <span class="given-name"><?php echo $row['given-name'] ?></span>
                            <span class="family-name"><?php echo $row['family-name'] ?></span>
                            <span class="location"><?php echo $row['location'] ?></span>
                            <span class="industry"><?php echo $row['industry'] ?></span>
                            <dd class="current-content">
                                <span><?php echo $row['headline'] ?></span>
                            </dd>
                            <a href="<?php echo $row['url'] ?>"></a>
                        </li>
                    <?php endwhile; ?>
                </ol>
                <?php

            endforeach;
    endif;
endif;

score 2 · Accepted Answer

作为一个一般概念，我会推荐一些东西：

正如其他人所提到的，“干燥”或“不要重复自己”是一个很好的概念。如果您不止一次地做某事，那么它很可能值得拥有自己的功能或可以简化。
虽然通常应用于面向对象的编程，但“单一职责原则”可以很好地应用于函数以提高它们的可维护性，但您也应该避免仅仅因为可以创建函数（函数调用需要开销）。最终，函数的集合通常以可重用的类而告终。
“KISS”或“保持简单，愚蠢”（注意：最好将其视为自我引用的“愚蠢”，例如当有人说，“我真是个白痴！”当他们想出办法时）——尽可能简化您的逻辑和代码。“最简单的解释通常是正确的。”

为了应用这些概念，我将如何重新构建（而不是我将如何编写）您的脚本：

查询是否有任何配置文件匹配不到 30 天，因为您正在更新配置文件时不存在或所有配置文件都超过 30 天。
如果您没有返回任何行：
- 查询是否有任何配置文件完全匹配，以确定更新与插入。
- 下载并解析页面。
- 保存新的/更新的记录。
- 保留您解析的匹配项，而不是从数据库中下载您刚刚插入/更新的内容。
最后，显示您的匹配项（无论它们来自数据库还是解析）。

通过以这种方式重组代码：

您消除了大部分重复和潜在的混乱
您简化了代码路径
您对逻辑组件进行分组（搜索、解析/存储（如有必要）、显示）
您的所有 HTML 都可以放在脚本的末尾，这样更具可读性（或者可以很容易地放在单独的文件中）
函数有意义的地方会更加明显，例如解析循环。

最后一点——当您处理来自“未知来源”（用户、网站、提供的文件等）的数据时，您应该强调安全性。虽然这是一个好主意addslashes()，urlencode()但有许多资源可以帮助您了解如何避免 SQL 注入、跨站点脚本和其他潜在威胁。代码中的一个风险示例是使用 $_REQUEST 而不转义数据库查询。

php - 重构技巧

1 回答 1

Related

Reference