Вы перепутались код функции, так что не работает. Я выделяю некоторые моменты, как вы можете улучшить его, что поможет вам не только удалить ошибки, но и предотвратить подобные ошибки в будущем, изменив способ написания кода.
В этом обсуждении я оставляю тему правильной кодировки значений в сторону, что может привести к ошибкам и дефектам (SQL-инцекция). Просто скажите, вы должны добавить их самостоятельно.
Основная задача этой функции - построить условный SQL-запрос и выполнить его. Поэтому в первую очередь переместить здание запроса из функции в зависимости от его собственной:
private function buildReadLocationSQL($countrycode)
{
$SQLPattern = "SELECT location AS loc FROM currency WHERE countrycode = '%s'";
return sprintf($SQLPattern, $countrycode);
}
Затем вам нужно выяснить, что $countrycode должно быть. Здесь вы используете два строковых значения, чтобы описать, что вам нужно. Почему бы не назвать функции вместо этого?
public function readLocationFrom()
{
...
}
public function readLocationTo()
{
...
}
Имея две функции здесь ясно, что они делают, и вы не можете делать какие-либо ошибки при вызове (вызов) их.
Недостающая часть предназначена для инкапсуляции соединения с базой данных и запроса. Это имеет смысл в любом случае, потому что вы хотите только подключиться к базе данных, как только приложение будет запущено , а не по каждому запросу. Подключение к базе данных дорого.Кроме того, это облегчает расширение вашего класса, делая код более модульным.
Итак, давайте немного изменим класс базы данных, чтобы он хранил это соединение (вам не нужно явно закрывать его, PHP сделает это для вас, когда закончится скрипт) и предложить функцию для запроса к базе данных :
class Database
{
private $connection;
private function connect()
{
$connection = mysqli_connect(DB_HOST, DB_USER, DB_PASSWORD, DB_NAME);
if ($connection === FALSE)
{
throw new RuntimeException(sprintf('Could not connect to Mysql database: %s', mysqli_connect_error()));
}
$this->connection = $connection;
}
private function query($sql)
{
if (!$this->connection) $this->connect(); // lazy connect to the DB
$result = mysqli_query($this->connection, $sql);
if (!$result)
{
throw new RuntimeException(sprintf('Could not query the Mysql database: %s', mysqli_error($this->connection)));
}
return $result;
}
...
}
Эти две новые частные функции заботятся о конкретных подключениях к базе данных и запросах. По мере того как вы добавляете все больше и больше функций в свой класс базы данных, вы можете легко их повторно использовать. Также эти два являются частными (как участник $connection), потому что вы хотите скрыть данные от остальной части вашего приложения.
Теперь это необходимо объединить с функциями readLocationFrom и readLocationTo, чтобы заставить его работать. Поскольку обе функции выполняют аналогичные функции, мы создаем еще одну частную функцию, которая инкапсулирует и то, и другое похоже на вашу исходную логику функций.
Однако на этот раз мы позаботимся о том, чтобы переданные аргументы содержали только значения, которые имеют смысл (предварительные проверки условий). Кроме того, функция является частным, так что она не может быть вызвана другим кодом, который уменьшает возможность вызова неправильно из-за пределы класса (он не может быть вызвана извне):
private function readLocationConditional($what)
{
switch($what)
{
case 'from':
$countrycode = $this->fromcurr;
break;
case 'to':
$countrycode = $this->tocurr;
break;
default:
throw new InvalidArgumentException(sprintf('Invalid value for $what ("%s").', $what));
}
$sql = $this->buildReadLocationSQL($countrycode);
$result = $this->query($sql);
$array = mysqli_fetch_array($result);
$field = 'loc';
if (!isset($array[$field]))
{
throw new UnexpectedValueException(sprintf('Row does not contain %s (contains %s)', $field, implode(',', array_keys($array))));
}
$location = $array[$field];
return $location;
}
Так давайте более подробно рассмотрим, что сделано по-разному с исходной функцией: входные значения проверены и условная переменная назначена $countrycode. Это помогает держать вещи в стороне. В случае, если задано какое-то «неопределенное» значение для $what, генерируется исключение. Это немедленно сообщит вам, если вы неправильно использовали эту функцию.
Затем для создания SQL используется новая функция buildReadLocationSQL, для запуска запроса используется новая функция query. В конце данные, возвращаемые из базы данных, снова проверяются и генерируется исключение, если необходимое поле отсутствует. Это немедленно сообщит вам, если что-то не так с возвращенными данными.
Это теперь легко сочетать с двумя новыми публичными функциями, которые являются фактическими обертками для новой частной функции readLocationConditional. Так давайте это все вместе:
public function readLocationFrom()
{
return $this->readLocationConditional('from');
}
public function readLocationTo()
{
return $this->readLocationConditional('to');
}
Итак, вы переписали класс базы данных, чтобы сделать его более гибким и улучшение функционирования. Поместите это вместе, и все.
Первым ключевым моментом здесь является то, что вы правильно проверяете условия ошибки. Я использовал исключения вместо die, потому что я думаю, что это более дружественный для разработчиков. Вы можете сделать это с помощью операторов die или значений ошибок и т. Д. Но Исключения не только сообщают вам сообщение, но и где что-то случилось, можно поймать (вы не можете поймать die) и можете иметь другой тип, поэтому они несут гораздо более полезную информацию, если что-то пойдет не так.
Второй ключевой момент заключается в том, что вы упрощаете свой код. Чем проще ваш поток кода, тем проще его кодировать. Вложение вещей, которые покрывают одно и то же, в собственные функции, очень продуктивно. Вы можете писать их, когда будете продолжать, пока вы не разделяете вещи. И еще один совет: чем меньше параметров имеет функция (в идеале: ноль, которая не всегда возможна), тем меньше вы можете ошибаться и меньше входных значений, которые необходимо проверить.
Просто сказать: вы изначально сделали это так или иначе, но не полностью.У вас было только одно место, где вы отметили ошибку (соединение с базой данных mysql), но не для самого запроса, а не для возвращаемого значения из запроса, а не для параметра функции. Вводя все в свои небольшие функции, легче увидеть, что нужно проверять и обрабатывать детали.
И последнее, но не в последнюю очереди, новое использование, я сделал имена более говоря (nstd о сокре evrthng):
$amount = "1.00";
$currencyFrom = "USD";
$currencyTo = "GBP";
$db = new Database($currencyFrom, $currencyTo);
echo $db->readLocationFrom(), $db->readLocationTo();
Надеется, что это помогает. И не бойтесь писать некоторые строки кода до тех пор, пока это помогает предотвратить ошибки (это ваш код, использовать его как инструмент для вас) и с учетом модульности. В конце вы увидите, что это меньше кода и меньше проблем. Поиск фактической части, которую вы совершили ошибку, также является хорошим местом для начала.
Обсуждение ошибок; Я видел следующий:
elseif ($toFrom == 'to');
^
If -блоки могут быть сложными, реструктуризация коды помогает здесь. Уменьшите использование if и особенно elseif. Перемещение частей в собственные функции помогает снизить сложность. Меньшая сложность, меньше ошибок (мы люди).
$query2 = ("...");
^ ^
Эта скобка является излишней, может быть знаком только для копирования кода из предыдущего вызова функции. Позаботьтесь.
Просто поместив палец на эти ошибки, вы не увидите всю картину, и только исправление этого не даст вам дальнейших шагов. Поэтому лучше распознать ошибки и понять, почему они были сделаны, а затем подумать о том, как их предотвратить.
Мы должны «просто знать», что делает «readLocation()»? Код черного ящика не способствует отладке ... –
Пожалуйста, покажите $ from и $ to. – ZeroSuf3r
Поскольку в этом вопросе нет актуального вопроса, я собираюсь использовать свое дикое воображение, чтобы определить, что вы действительно пытаетесь спросить, существуют ли единороги. На что я отвечу: *** «Пока вы верите, Марк. Пока вы верите». *** – rdlowrey