Skip to main content
Code Review

Return to Answer

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

This:

$tables = array(
 "day" => "p_day", 
 "month" => "p_month"
 ... etc. .....
);
$table = $tables[$user_table];
if(!$table) {
 die(json_encode(array("error" => "bad table name")));
}

will throw a notice if $user_table is not a valid array key, something you should have already tested. Rewrite as:

$tables = array(
 "day" => "p_day", 
 "month" => "p_month"
 ... etc. .....
);
if(!array_key_exists($user_table, $tables)) {
 die(json_encode(array("error" => "bad table name")));
}
$table = $tables[$user_table];

I really hate it when functions die(), and in your case there's no point in that. You could simply do:

if(!$table) {
 return json_encode(array("error" => "bad table name"));
}

since the function is expected to return a json formatted string. If you really want to die() you should do that where you call the function and the returned string is an error. Or you could just return false when an error occurs and the raw array when everything works, and when calling the function do something like:

$result = json_encode(getReport("some_table"));

That way getReport() will be useful even when you don't need json encoded output. But that's up to you and how you actually use it.

As @LokiAstari mentions mentions mysql_* functions are deprecated and should be avoided. I would skip mysqli_* functions too and use PDO which will give you the extra bonus of switching to another database engine if you ever need to and it has a nice OO interface.

For everything else, I'm with @LokiAstari.

This:

$tables = array(
 "day" => "p_day", 
 "month" => "p_month"
 ... etc. .....
);
$table = $tables[$user_table];
if(!$table) {
 die(json_encode(array("error" => "bad table name")));
}

will throw a notice if $user_table is not a valid array key, something you should have already tested. Rewrite as:

$tables = array(
 "day" => "p_day", 
 "month" => "p_month"
 ... etc. .....
);
if(!array_key_exists($user_table, $tables)) {
 die(json_encode(array("error" => "bad table name")));
}
$table = $tables[$user_table];

I really hate it when functions die(), and in your case there's no point in that. You could simply do:

if(!$table) {
 return json_encode(array("error" => "bad table name"));
}

since the function is expected to return a json formatted string. If you really want to die() you should do that where you call the function and the returned string is an error. Or you could just return false when an error occurs and the raw array when everything works, and when calling the function do something like:

$result = json_encode(getReport("some_table"));

That way getReport() will be useful even when you don't need json encoded output. But that's up to you and how you actually use it.

As @LokiAstari mentions mysql_* functions are deprecated and should be avoided. I would skip mysqli_* functions too and use PDO which will give you the extra bonus of switching to another database engine if you ever need to and it has a nice OO interface.

For everything else, I'm with @LokiAstari.

This:

$tables = array(
 "day" => "p_day", 
 "month" => "p_month"
 ... etc. .....
);
$table = $tables[$user_table];
if(!$table) {
 die(json_encode(array("error" => "bad table name")));
}

will throw a notice if $user_table is not a valid array key, something you should have already tested. Rewrite as:

$tables = array(
 "day" => "p_day", 
 "month" => "p_month"
 ... etc. .....
);
if(!array_key_exists($user_table, $tables)) {
 die(json_encode(array("error" => "bad table name")));
}
$table = $tables[$user_table];

I really hate it when functions die(), and in your case there's no point in that. You could simply do:

if(!$table) {
 return json_encode(array("error" => "bad table name"));
}

since the function is expected to return a json formatted string. If you really want to die() you should do that where you call the function and the returned string is an error. Or you could just return false when an error occurs and the raw array when everything works, and when calling the function do something like:

$result = json_encode(getReport("some_table"));

That way getReport() will be useful even when you don't need json encoded output. But that's up to you and how you actually use it.

As @LokiAstari mentions mysql_* functions are deprecated and should be avoided. I would skip mysqli_* functions too and use PDO which will give you the extra bonus of switching to another database engine if you ever need to and it has a nice OO interface.

For everything else, I'm with @LokiAstari.

Source Link
yannis
  • 2.1k
  • 1
  • 19
  • 38

This:

$tables = array(
 "day" => "p_day", 
 "month" => "p_month"
 ... etc. .....
);
$table = $tables[$user_table];
if(!$table) {
 die(json_encode(array("error" => "bad table name")));
}

will throw a notice if $user_table is not a valid array key, something you should have already tested. Rewrite as:

$tables = array(
 "day" => "p_day", 
 "month" => "p_month"
 ... etc. .....
);
if(!array_key_exists($user_table, $tables)) {
 die(json_encode(array("error" => "bad table name")));
}
$table = $tables[$user_table];

I really hate it when functions die(), and in your case there's no point in that. You could simply do:

if(!$table) {
 return json_encode(array("error" => "bad table name"));
}

since the function is expected to return a json formatted string. If you really want to die() you should do that where you call the function and the returned string is an error. Or you could just return false when an error occurs and the raw array when everything works, and when calling the function do something like:

$result = json_encode(getReport("some_table"));

That way getReport() will be useful even when you don't need json encoded output. But that's up to you and how you actually use it.

As @LokiAstari mentions mysql_* functions are deprecated and should be avoided. I would skip mysqli_* functions too and use PDO which will give you the extra bonus of switching to another database engine if you ever need to and it has a nice OO interface.

For everything else, I'm with @LokiAstari.

lang-php

AltStyle によって変換されたページ (->オリジナル) /