Гость
Войти | Профиль | Очистить
Целевая тема:
Создать новую тему:
Автор:
Действия ...
Форумы / PHP, Perl, Python [игнор отключен] [закрыт для гостей] / Прокритикуйте мой код / 8 сообщений из 8, страница 1 из 1
12.02.2013, 23:53
    #38148686
ccoder
ccoder
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Прокритикуйте мой код
Много чего тут "ламерского" (особенно в php), поэтому что я хотелбы узнать то как это сделалибы профессионалы.
Так что пинайте где что неправильно и как научится делать такие задания быстро(и качественно).

Вот такая база данных и задание (составленные не мной)
авторDatabase

1.System Users
1.1. User ID
1.2. Username
1.3. User Password
1.4. Email
1.5. Admin?
1.6. User status

2. Driver
2.1. Driver ID
2.2. Name
2.3. Surname
2.4. User ID (this way a driver related with System User)
2.5. Driver status
2.6. Driver photo

3.Trucks
3.1. Truck ID
3.2. Name
3.3. Number
3.4. Fuel rate L/ val. idle
3.5. Fuel rate L/ val. driving
3.6. Fuel rate L/ val. compressor
3.7. Fuel rate L/ val. idle, in terminal

4. Drivers assignments
4.1. Truck ID
4.2. Driver ID

5. Trip
5.1. Trip ID
5.2. Driver ID
5.3. Truck ID
5.4. Trip data (yyyy-mm-dd)
5.5. Route (text input)
5.6. Departure from terminal (hh:mm)
5.7. Speedometer value before departure from terminal (number input)
5.8. Arrival to client (hh:mm)
5.9. Unload length (hh:mm - length)
5.10. Departure from client (hh:mm)
5.11. Arrival to terminal (hh:mm)
5.12. Speedometer value after departure arrival
5.13. Length
5.14. Weight (tons)
5.15. Used fuel

Control

1.User control (for admins)
1.1. Right to create/edit system users and grand admin rights
1.2. Right to assing status to DISABLE - these users cannot login

2.Drivers control (for admins)
2.1. Right to create driver and enter their data and upload photo
2.2. Right to assign system user for driver
2.3. Right for driver to assign a truck
2.4. Right to assign to DISABLED – there drivers cannot login

3.Trucks control (for admins)
3.1. Right to create/edit trucks and enter their data
3.2. Right to assign a driver for truck
...

сделал и получил такие таблицы
http://img705.imageshack.us/img705/6491/15338610.png
и сделал php, для просмотра и добавления пользователей только
connection.php
Код: php
1.
2.
3.
4.
5.
6.
7.
8.
9.
10.
<?php
$host = 'localhost';
$username = 'root';
$password = '';
$database = 'trucks';

$connect = mysql_connect($host,$username,$password) or die(mysql_error());
mysql_select_db($database,$connect) or die(mysql_error());
mysql_set_charset('utf8');
?>


index.php
Код: php
1.
2.
3.
4.
5.
6.
7.
8.
9.
10.
11.
12.
13.
14.
15.
16.
17.
18.
19.
20.
21.
22.
23.
24.
25.
26.
27.
28.
29.
30.
31.
32.
33.
34.
35.
36.
37.
38.
39.
40.
41.
42.
43.
44.
45.
46.
47.
48.
49.
50.
51.
52.
53.
54.
55.
56.
57.
58.
59.
60.
61.
62.
63.
64.
65.
66.
67.
68.
69.
70.
71.
72.
73.
74.
75.
76.
77.
78.
79.
80.
81.
82.
83.
84.
85.
86.
87.
88.
89.
90.
91.
92.
93.
94.
95.
96.
97.
98.
99.
100.
101.
102.
103.
104.
105.
106.
107.
108.
109.
110.
111.
112.
113.
114.
115.
116.
117.
118.
119.
120.
121.
122.
123.
124.
125.
126.
127.
128.
129.
130.
131.
132.
133.
134.
135.
136.
137.
138.
139.
140.
141.
142.
143.
144.
145.
146.
147.
148.
149.
150.
151.
152.
153.
154.
155.
156.
157.
158.
159.
160.
161.
162.
163.
164.
165.
166.
167.
168.
169.
170.
171.
172.
173.
174.
175.
176.
177.
178.
179.
180.
181.
182.
183.
184.
185.
186.
187.
188.
189.
190.
191.
192.
193.
194.
195.
196.
197.
198.
199.
200.
201.
202.
203.
204.
205.
206.
207.
208.
209.
210.
211.
212.
213.
214.
215.
216.
217.
218.
219.
220.
221.
222.
223.
224.
225.
226.
227.
228.
229.
230.
231.
232.
233.
234.
235.
236.
237.
238.
239.
240.
241.
242.
243.
244.
245.
246.
247.
248.
249.
250.
251.
252.
253.
254.
255.
256.
257.
258.
259.
260.
261.
262.
263.
264.
265.
266.
267.
268.
269.
<?php
include ('connection.php');
?>

<!DOCTYPE html>
<html>
<head>
    <title>System Users</title>
    <link rel="stylesheet" type="text/css" href="./css/theme.css"/>
</head>
<body>

<?php

$params['name'] = "";
$params['surname'] = "";
$params['password'] = "";
$params['email'] = "";
$params['is_admin'] = "";
$params['status'] = "";

if (isset($_POST['add'])) {

    if (!isset($_POST['status'])) {
        $_POST['status'] = "";
    }

    if (!get_magic_quotes_gpc()) {
        $params['name'] = addslashes($_POST['name']);
        $params['surname'] = addslashes($_POST['surname']);
        $params['password'] = addslashes($_POST['password']);
        $params['email'] = addslashes($_POST['email']);
        $params['is_admin'] = addslashes($_POST['is_admin']);
        $params['status'] = addslashes($_POST['status']);
    } else {
        $params['name'] = $_POST['name'];
        $params['surname'] = $_POST['surname'];
        $params['password'] = $_POST['password'];
        $params['email'] = $_POST['email'];
        $params['is_admin'] = $_POST['is_admin'];
        $params['status'] = $_POST['status'];
    }

    if (empty($params['name']) || empty($params['surname']) || empty($params['password']) || empty($params['email']) || !isset($params['is_admin']) || empty($params['status'])) {
        if (empty($params['name'])) {
            $err['name'] = "Name not entered";
        }
        if (empty($params['surname'])) {
            $err['surname'] = "Surname not entered";
        }
        if (empty($params['password'])) {
            $err['password'] = "Password not entered";
        }
        if (empty($params['email'])) {
            $err['email'] = "Email not entered";
        }
        if (!isset($params['is_admin'])) {
            $err['is_admin'] = "Not chosen";
        }
        if (empty($params['status'])) {
            $err['status'] = "Status not chosen";
        }
    } else {
        if (strlen($params['name']) > 45) {
            $err['name'] = "Length not more 45 symbols";
        } else if (strlen($params['name']) > 45) {
            $err['surname'] = "Length not more 45 symbols";
        } else if (strlen($params['password']) > 45) {
            $err['password'] = "Length not more 45 symbols";
        } else if (strlen($params['email']) > 45) {
            $err['email'] = "Length not more 45 symbols";
        } else if (!filter_var($params['email'], FILTER_VALIDATE_EMAIL)) {
            $err['email'] = "Wrong email format";
        } else if (!in_array($params['status'], array("ENABLED", "DISABLED"))) {
            $err['status'] = "wrong status";
        } else {

            $query = sprintf("INSERT INTO `system_users` (`name`, `surname`, `password`, `email`, `is_admin`, `status`) VALUES ('%s','%s','%s','%s',%d,'%s')",
                mysql_real_escape_string($params['name']),
                mysql_real_escape_string($params['surname']),
                mysql_real_escape_string($params['password']),
                mysql_real_escape_string($params['email']),
                $params['is_admin'],
                mysql_real_escape_string($params['status'])
            );

            $result = @mysql_query("$query") or die(mysql_error());

            $_POST['add'] = null;
            $message = "Driver successfully added";

            $params['name'] = "";
            $params['surname'] = "";
            $params['password'] = "";
            $params['email'] = "";
            $params['is_admin'] = "";
            $params['status'] = "";

        }

    }
}
?>



<h3>System users</h3>

<table border="1">
    <th>Username</th>
    <th>Surname</th>
    <th>Password</th>
    <th>Email</th>
    <th>Is admin</th>
    <th>Status</th>

    <?php
    $query1 = "SELECT * FROM system_users";
    $result1 = @mysql_query("$query1") or die(mysql_error());

    $boolean = array(
        0 => "No",
        1 => "Yes",
        "" => ""
    );

    while ($row1 = mysql_fetch_array($result1)) {
        echo "<tr>";

        echo "<td>" . $row1['name'] . "</td>";
        echo "<td>" . $row1['surname'] . "</td>";
        echo "<td>" . $row1['password'] . "</td>";
        echo "<td>" . $row1['email'] . "</td>";
        echo "<td>" . $boolean[$row1['is_admin']] . "</td>";
        echo "<td>" . $row1['status'] . "</td>";
        echo "</tr>";
    }
    ?>
</table>

<br>

<?php

if (isset($message)) {
    echo "<div class=\"message\">" . $message . "</div>";
}

?>


<br>


<form method="post">

    <table border="0">
        <tr>
            <td>Name:</td>
            <td><input type="text" name="name" value="<?php echo $params['name']?>"></td>
            <?php
            if (isset($err['name'])) {
                echo "<td class=\"error\">" . $err['name'] . "</td>";
            } else {
                echo "<td>* length not more 45 symbols</td>";
            }
            ?>
        </tr>
        <tr>
            <td>Surname:</td>
            <td><input type="text" name="surname" value="<?php echo $params['surname']?>"></td>
            <?php
            if (isset($err['surname'])) {
                echo "<td class=\"error\">" . $err['surname'] . "</td>";
            } else {
                echo "<td>* length not more 45 symbols</td>";
            }
            ?>
        </tr>
        <tr>
            <td>Password:</td>
            <td><input type="password" name="password" value="<?php echo $params['password']?>"></td>
            <?php
            if (isset($err['password'])) {
                echo "<td class=\"error\">" . $err['password'] . "</td>";
            } else {
                echo "<td>* length not more 45 symbols</td>";
            }
            ?>
        </tr>
        <tr>
            <td>Email:</td>
            <td><input type="text" name="email" value="<?php echo $params['email']?>"></td>
            <?php
            if (isset($err['email'])) {
                echo "<td class=\"error\">" . $err['email'] . "</td>";
            } else {
                echo "<td>* length not more 45 symbols and right email format.</td>";
            }
            ?>
        </tr>
        <tr>
            <td>Admin:</td>

            <td>

                <p>
                    <label>
                        <input type="radio" name="is_admin" value="1" id="isAdmin_0"
                            <?php if ($params['is_admin']) {
                            echo 'checked = "checked"';
                        } ?>/>
                        Admin</label>

                    <label>
                        <input type="radio" name="is_admin" value="0" id="isAdmin_1"
                            <?php if (!$params['is_admin']) {
                            echo 'checked = "checked"';
                        } ?>/>
                        User</label>
                    <br/>
                </p>


            </td>

            <?php
            if (isset($err['is_admin'])) {
                echo "<td class=\"error\">" . $err['is_admin'] . "</td>";
            }
            ?>
        </tr>


        <tr>
            <td>Status:</td>
            <td>
                <select name="status" size="3" id="status">
                    <option value="ENABLED" <?php if ($params['status'] == "ENABLED") {
                        echo 'selected = "selected"';
                    } ?> >Can login
                    </option>
                    <option value="DISABLED" <?php if ($params['status'] == "DISABLED") {
                        echo 'selected = "selected"';
                    } ?> >Cannot login
                    </option>
                </select>
            </td>



            <?php
            if (isset($err['status'])) {
                echo "<td class=\"error\">" . $err['status'] . "</td>";
            }
            ?>


        </tr>
    </table>

    <input type="submit" name="add" id="add" value="Add">


</form>


</body>
</html>
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
13.02.2013, 00:19
    #38148693
vkle
vkle
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Прокритикуйте мой код
Обычная "лапша". Нудновато, но хорошо читаемо и понятно. Пока параметров всего шесть, разумеется :-)
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
13.02.2013, 00:46
    #38148699
ccoder
ccoder
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Прокритикуйте мой код
ну давайте как не лапшу сделать
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
13.02.2013, 01:40
    #38148726
ScareCrow
ScareCrow
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Прокритикуйте мой код
http://twig.sensiolabs.org/
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
13.02.2013, 02:06
    #38148738
vkle
vkle
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Прокритикуйте мой код
ccoder , а чем плохо то что есть сейчас?

Для странички-одиночки, единственной целью которой будет редкая запись каких-то реквизитов в какую-то служебную базу - сойдет вполне. Один раз написали, чуток поправили в процессе притирки - и можно забыть. Поддерживать в случае нужды просто - любой новичек разберется. Это несомненные плюсы, на мой взгляд.

Теперь давайте предположим, что потребовалось добавить еще одно поле. Допустим, номер телефона. Что будем делать? Правильно - копипаст, правка, копипаст, правка, еще раз копипаст, правка и опять копипаст, правка. Потом искать что пропустили. На следующий день понадобилось еще и номер офиса добавить. Повторяем те же повторяющиеся операции. Тупо? Да. И вероятность допустить ошибку при правке все же есть немалая.

Глядя на повторяющийся код, не возникают ли мысли упихать эти шесть элементов (включая сообщения об ошибках и условия/ограничения на значения) в массив и гонять по нему цикл по мере надобности? Потребовалось добавить поле - вся работа сведется к правке одного массива. Ну, как вариант, конечно. И исключительно для данного случая.

А если верстку/дизайн поправить придется?
А если подобных страниц будет не одна, а пять, десять, сто?
А если где-то будет массив GET, а не POST (в отчетах, например)?
А если.... (напишите свой вариант)

Для каждого случая будут различные альтернативные способы реализации и свои плюсы и минусы.

Может быть, для множества страниц есть смысл использовать шаблонизатор. Но это потребует знания его синтаксиса, функций. А значит, и более высокой квалификации персонала поддержки. Для одной странички это будет минусом, для десяти - плюсом, ибо не придется в 150 мест копипасть какой-нить класс для выравнивания текста в первой ячейке таблицы.

Ну, как-то так... Все зависит от потребностей.

PS: Лень разбираться, но с блоком if (!get_magic_quotes_gpc()) вроде что-то не то. В базе хотите хранить значения с избытком слешей, если правильно понял. Сомнительно как-то.
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
13.02.2013, 02:12
    #38148741
ScareCrow
ScareCrow
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Прокритикуйте мой код
авторо с блоком if (!get_magic_quotes_gpc()) вроде что-то не то
Код: php
1.
$params['email'] = addslashes($_POST['email']);


смеялсо.
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
13.02.2013, 11:50
    #38149112
ccoder
ccoder
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Прокритикуйте мой код
авторМожет быть, для множества страниц есть смысл использовать шаблонизатор. Но это потребует знания его синтаксиса, функций. А значит, и более высокой квалификации персонала поддержки. Для одной странички это будет минусом, для десяти - плюсом, ибо не придется в 150 мест копипасть какой-нить класс для выравнивания текста в первой ячейке таблицы.
А с zend что можно?
И с объектным программированием?
Т.е. допустим вам дали такое задание, технология не важна но всё на php, то как бы вы это сделали?
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
13.02.2013, 12:55
    #38149274
Ренат
Ренат
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Прокритикуйте мой код
ccoderавторМожет быть, для множества страниц есть смысл использовать шаблонизатор. Но это потребует знания его синтаксиса, функций. А значит, и более высокой квалификации персонала поддержки. Для одной странички это будет минусом, для десяти - плюсом, ибо не придется в 150 мест копипасть какой-нить класс для выравнивания текста в первой ячейке таблицы.
А с zend что можно?
И с объектным программированием?
Т.е. допустим вам дали такое задание, технология не важна но всё на php, то как бы вы это сделали?
Один из вариантов:
1. Сделал бы проект на Yii и в нем попросил Yii сгенерить все что вы написали за меня.
2. Сделал бы проект на Symfony и в нем попросил Symfony сгенерить все что вы написали за меня.
etc
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
Форумы / PHP, Perl, Python [игнор отключен] [закрыт для гостей] / Прокритикуйте мой код / 8 сообщений из 8, страница 1 из 1
Найденые пользователи ...
Разблокировать пользователей ...
Читали тему (0):
Читали форум (0):
Пользователи онлайн (0):
Польз. соглашение  
Полит. конфиден.  
созд. / загр.: 359ms
Закрыть
start [/forum/topic.php?fid=23&mobile=1&tid=1464131]:
 0ms
get settings:
 8ms
get forum list:
 19ms
check forum access:
 3ms
check topic access:
 3ms
track hit:
 52ms
get topic data:
 9ms
get forum data:
 2ms
get page messages:
 43ms
get tp. blocked users:
 1ms
others: 219ms
total:  359ms
x
x
Закрыть


Просмотр
0 / 0
Close
Debug Console [Select Text]