Skip to main content
Code Review

Return to Answer

replaced http://programmers.stackexchange.com/ with https://softwareengineering.stackexchange.com/
Source Link

Poor cohesion

It's called logical cohesion when a routine that does multiple things of different logic. Your function will either insert records, or select records, or update records. These are very different operations, and their implementations should be in separate functions.

If you refactored this way, it would be much better:

if ($Type == "INSERT") performInsert(...)
elseif ($Type == "SELECT") performSelect(...)
elseif ($Type == "UPDATE") performUpdate(...)
// ...

In this form, by using an if-else chain to decide the next action and delegating to appropriate specialized functions, this function will have functional cohesion, which is the best kind of cohesion, characterized by a routine having a single purpose, in this case orchestrating database operations.

Mutually exclusive if

Notice that in the previous point I chained the conditions with elseif. In your code there are many if conditions that cannot happen together, so they should have been elseif.

Poor naming

I think, the common convention is to only use PascalCase for class names in PHP. Function names and variable names are usually either snake_case or camelCase. See also this related post. See also this related post.


It seems that $Param is expected to be an array, just like $Values. To indicate this fact better, I suggest making it plural, to $Params.


There are several problems with these names:

$Lenght = count($Values);
$Lenght1 = count($Param);
  • The correct spelling is "length"
  • Numbering in variable names is considered bad practice
  • The "first" length being $Lenght, and the "second" length being $Lenght1 is just odd

I suggest renaming like this instead:

$valueCount = count($Values);
$paramCount = count($Param);

You should find better alternatives for the other numbered variables too, like $Rez1.

Poor cohesion

It's called logical cohesion when a routine that does multiple things of different logic. Your function will either insert records, or select records, or update records. These are very different operations, and their implementations should be in separate functions.

If you refactored this way, it would be much better:

if ($Type == "INSERT") performInsert(...)
elseif ($Type == "SELECT") performSelect(...)
elseif ($Type == "UPDATE") performUpdate(...)
// ...

In this form, by using an if-else chain to decide the next action and delegating to appropriate specialized functions, this function will have functional cohesion, which is the best kind of cohesion, characterized by a routine having a single purpose, in this case orchestrating database operations.

Mutually exclusive if

Notice that in the previous point I chained the conditions with elseif. In your code there are many if conditions that cannot happen together, so they should have been elseif.

Poor naming

I think, the common convention is to only use PascalCase for class names in PHP. Function names and variable names are usually either snake_case or camelCase. See also this related post.


It seems that $Param is expected to be an array, just like $Values. To indicate this fact better, I suggest making it plural, to $Params.


There are several problems with these names:

$Lenght = count($Values);
$Lenght1 = count($Param);
  • The correct spelling is "length"
  • Numbering in variable names is considered bad practice
  • The "first" length being $Lenght, and the "second" length being $Lenght1 is just odd

I suggest renaming like this instead:

$valueCount = count($Values);
$paramCount = count($Param);

You should find better alternatives for the other numbered variables too, like $Rez1.

Poor cohesion

It's called logical cohesion when a routine that does multiple things of different logic. Your function will either insert records, or select records, or update records. These are very different operations, and their implementations should be in separate functions.

If you refactored this way, it would be much better:

if ($Type == "INSERT") performInsert(...)
elseif ($Type == "SELECT") performSelect(...)
elseif ($Type == "UPDATE") performUpdate(...)
// ...

In this form, by using an if-else chain to decide the next action and delegating to appropriate specialized functions, this function will have functional cohesion, which is the best kind of cohesion, characterized by a routine having a single purpose, in this case orchestrating database operations.

Mutually exclusive if

Notice that in the previous point I chained the conditions with elseif. In your code there are many if conditions that cannot happen together, so they should have been elseif.

Poor naming

I think, the common convention is to only use PascalCase for class names in PHP. Function names and variable names are usually either snake_case or camelCase. See also this related post.


It seems that $Param is expected to be an array, just like $Values. To indicate this fact better, I suggest making it plural, to $Params.


There are several problems with these names:

$Lenght = count($Values);
$Lenght1 = count($Param);
  • The correct spelling is "length"
  • Numbering in variable names is considered bad practice
  • The "first" length being $Lenght, and the "second" length being $Lenght1 is just odd

I suggest renaming like this instead:

$valueCount = count($Values);
$paramCount = count($Param);

You should find better alternatives for the other numbered variables too, like $Rez1.

Pretty sure this is what you meant to say.
Source Link
Veedrac
  • 9.8k
  • 23
  • 38

Poor cohesion

It's called logical cohesion when a routine that does multiple things of different logic. Your function will either insert records, or select records, or update records. These are very different operations, and their implementationimplementations should be in a single functionseparate functions.

If you refactored this way, it would be much better:

if ($Type == "INSERT") performInsert(...)
elseif ($Type == "SELECT") performSelect(...)
elseif ($Type == "UPDATE") performUpdate(...)
// ...

In this form, by using an if-else chain to decide the next action and delegating to appropriate specialized functions, this function will have functional cohesion, which is the best kind of cohesion, characterized by a routine having a single purpose, in this case orchestrating database operations.

Mutually exclusive if

Notice that in the previous point I chained the conditions with elseif. In your code there are many if conditions that cannot happen together, so they should have been elseif.

Poor naming

I think, the common convention is to only use PascalCase for class names in PHP. Function names and variable names are usually either snake_case or camelCase. See also this related post.


It seems that $Param is expected to be an array, just like $Values. To indicate this fact better, I suggest making it plural, to $Params.


There are several problems with these names:

$Lenght = count($Values);
$Lenght1 = count($Param);
  • The correct spelling is "length"
  • Numbering in variable names is considered bad practice
  • The "first" length being $Lenght, and the "second" length being $Lenght1 is just odd

I suggest renaming like this instead:

$valueCount = count($Values);
$paramCount = count($Param);

You should find better alternatives for the other numbered variables too, like $Rez1.

Poor cohesion

It's called logical cohesion when a routine that does multiple things of different logic. Your function will either insert records, or select records, or update records. These are very different operations, and their implementation should be in a single function.

If you refactored this way, it would be much better:

if ($Type == "INSERT") performInsert(...)
elseif ($Type == "SELECT") performSelect(...)
elseif ($Type == "UPDATE") performUpdate(...)
// ...

In this form, by using an if-else chain to decide the next action and delegating to appropriate specialized functions, this function will have functional cohesion, which is the best kind of cohesion, characterized by a routine having a single purpose, in this case orchestrating database operations.

Mutually exclusive if

Notice that in the previous point I chained the conditions with elseif. In your code there are many if conditions that cannot happen together, so they should have been elseif.

Poor naming

I think, the common convention is to only use PascalCase for class names in PHP. Function names and variable names are usually either snake_case or camelCase. See also this related post.


It seems that $Param is expected to be an array, just like $Values. To indicate this fact better, I suggest making it plural, to $Params.


There are several problems with these names:

$Lenght = count($Values);
$Lenght1 = count($Param);
  • The correct spelling is "length"
  • Numbering in variable names is considered bad practice
  • The "first" length being $Lenght, and the "second" length being $Lenght1 is just odd

I suggest renaming like this instead:

$valueCount = count($Values);
$paramCount = count($Param);

You should find better alternatives for the other numbered variables too, like $Rez1.

Poor cohesion

It's called logical cohesion when a routine that does multiple things of different logic. Your function will either insert records, or select records, or update records. These are very different operations, and their implementations should be in separate functions.

If you refactored this way, it would be much better:

if ($Type == "INSERT") performInsert(...)
elseif ($Type == "SELECT") performSelect(...)
elseif ($Type == "UPDATE") performUpdate(...)
// ...

In this form, by using an if-else chain to decide the next action and delegating to appropriate specialized functions, this function will have functional cohesion, which is the best kind of cohesion, characterized by a routine having a single purpose, in this case orchestrating database operations.

Mutually exclusive if

Notice that in the previous point I chained the conditions with elseif. In your code there are many if conditions that cannot happen together, so they should have been elseif.

Poor naming

I think, the common convention is to only use PascalCase for class names in PHP. Function names and variable names are usually either snake_case or camelCase. See also this related post.


It seems that $Param is expected to be an array, just like $Values. To indicate this fact better, I suggest making it plural, to $Params.


There are several problems with these names:

$Lenght = count($Values);
$Lenght1 = count($Param);
  • The correct spelling is "length"
  • Numbering in variable names is considered bad practice
  • The "first" length being $Lenght, and the "second" length being $Lenght1 is just odd

I suggest renaming like this instead:

$valueCount = count($Values);
$paramCount = count($Param);

You should find better alternatives for the other numbered variables too, like $Rez1.

added 1059 characters in body
Source Link
RoboSanta
  • 363
  • 1
  • 8

Poor cohesion

It's called logical cohesion when a routine that does multiple things of different logic. Your function will either insert records, or select records, or update records. These are very different operations, and their implementation should be in a single function.

If you refactored this way, it would be much better:

if ($Type == "INSERT") performInsert(...)
elseif ($Type == "SELECT") performSelect(...)
elseif ($Type == "UPDATE") performUpdate(...)
// ...

In this form, by using an if-else chain to decide the next action and delegating to appropriate specialized functions, this function will have functional cohesion, which is the best kind of cohesion, characterized by a routine having a single purpose, in this case orchestrating database operations.

Mutually exclusive if

Notice that in the previous point I chained the conditions with elseif. In your code there are many if conditions that cannot happen together, so they should have been elseif.

Poor naming

I think, the common convention is to only use PascalCase for class names in PHP. Function names and variable names are usually either snake_case or camelCase. See also this related post.


It seems that $Param is expected to be an array, just like $Values. To indicate this fact better, I suggest making it plural, to $Params.


There are several problems with these names:

$Lenght = count($Values);
$Lenght1 = count($Param);
  • The correct spelling is "length"
  • Numbering in variable names is considered bad practice
  • The "first" length being $Lenght, and the "second" length being $Lenght1 is just odd

I suggest renaming like this instead:

$valueCount = count($Values);
$paramCount = count($Param);

You should find better alternatives for the other numbered variables too, like $Rez1.

I think, the common convention is to only use PascalCase for class names in PHP. Function names and variable names are usually either snake_case or camelCase. See also this related post.


It seems that $Param is expected to be an array, just like $Values. To indicate this fact better, I suggest making it plural, to $Params.


There are several problems with these names:

$Lenght = count($Values);
$Lenght1 = count($Param);
  • The correct spelling is "length"
  • Numbering in variable names is considered bad practice
  • The "first" length being $Lenght, and the "second" length being $Lenght1 is just odd

I suggest renaming like this instead:

$valueCount = count($Values);
$paramCount = count($Param);

You should find better alternatives for the other numbered variables too, like $Rez1.

Poor cohesion

It's called logical cohesion when a routine that does multiple things of different logic. Your function will either insert records, or select records, or update records. These are very different operations, and their implementation should be in a single function.

If you refactored this way, it would be much better:

if ($Type == "INSERT") performInsert(...)
elseif ($Type == "SELECT") performSelect(...)
elseif ($Type == "UPDATE") performUpdate(...)
// ...

In this form, by using an if-else chain to decide the next action and delegating to appropriate specialized functions, this function will have functional cohesion, which is the best kind of cohesion, characterized by a routine having a single purpose, in this case orchestrating database operations.

Mutually exclusive if

Notice that in the previous point I chained the conditions with elseif. In your code there are many if conditions that cannot happen together, so they should have been elseif.

Poor naming

I think, the common convention is to only use PascalCase for class names in PHP. Function names and variable names are usually either snake_case or camelCase. See also this related post.


It seems that $Param is expected to be an array, just like $Values. To indicate this fact better, I suggest making it plural, to $Params.


There are several problems with these names:

$Lenght = count($Values);
$Lenght1 = count($Param);
  • The correct spelling is "length"
  • Numbering in variable names is considered bad practice
  • The "first" length being $Lenght, and the "second" length being $Lenght1 is just odd

I suggest renaming like this instead:

$valueCount = count($Values);
$paramCount = count($Param);

You should find better alternatives for the other numbered variables too, like $Rez1.

Source Link
RoboSanta
  • 363
  • 1
  • 8
Loading
lang-php

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