Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

##Use a validation function

Use a validation function

##Many simple functions

Many simple functions

##Avoid repetition

Avoid repetition

##Use a validation function

##Many simple functions

##Avoid repetition

Use a validation function

Many simple functions

Avoid repetition

whitespace matters, as does code that parses.
Source Link
AD7six
  • 1.4k
  • 9
  • 11
public function submitCoil($data)
{
 ...
 if (!$this->validates($queryData['values'])) {
 return false;
 }
 return $db->insertFew("coils", $queryData['fields'], $queryData['values']);
}
protected function validates($values)
{
 $mandatoryFields = ['name', 'resistance', 'description', 'wraps'];
 $return = true;
 foreach($mandatoryFields as $field) {
 if (empty($values[$field])) {
 // Consider doing something like this so you can know/say
 // what the problem is.
 // $this->validationErrors[$field] = "This is empty";
 $return =false;= false;
 }
 }
 return $return;
}
public function submitCoil($data)
{
 $queryData = $this->translateData($data);
 if (!$this->validates($queryData['values'])) {
 return false;
 }
 return $this->insert($queryData);
}
protected function translateData($data)
{
 $uniqueID = $this->hash("id");
 return array([
 'fields' => array([
 'uniqueid',
 'uploaded',
 'name',
 'resistance',
 'wrapped',
 'category',
 'description',
 'wraps',
 'images',
 'wire_one',
 'wire_two',
 'wire_three',
 'wire_four',
 'wire_five',
 'wire_six'
 )],
 'values' => array([
 'uniqueID' => $uniqueID,
 'uploaded' => date("F j, Y", time()),
 'name' => $data['name'],
 'resistance' => $data['resistance'],
 'wrapped' => $data['wrapped'],
 'category' => $data['category'],
 'description' => $data['description'],
 'wraps' => $data['wraps'],
 'images' => "coils/" . $uniqueID . "/",
 'wire_one' => $data['wire_one'],
 'wire_two' => $data['wire_two'],
 'wire_three' => $data['wire_three'],
 'wire_four' => $data['wire_four'],
 'wire_five' => $data['wire_five'],
 'wire_six' => $data['wire_six']
 )]
 );];
}
protected function validates($values)
{
 $mandatoryFields = ['name', 'resistance', 'description', 'wraps'];
 foreach($mandatoryFields as $field) {
 if (empty($values[$field])) {
 return false;
 }
 }
 return true;
}
protected function insert($data)
{
 $db = Database::getInstance();
 return $db->insertFew("coils", $data['fields'], $data['values']);
}
$queryData = array([
 'uniqueid' => $uniqueID,
 'uploaded' => date("F j, Y", time()),
 'name' => $data['name'],
 'resistance' => $data['resistance'],
 'wrapped' => $data['wrapped'],
 'category' => $data['category'],
 'description' => $data['description'],
 'wraps' => $data['wraps'],
 'images' => "coils/" . $uniqueID . "/",
 'wire_one' => $data['wire_one'],
 'wire_two' => $data['wire_two'],
 'wire_three' => $data['wire_three'],
 'wire_four' => $data['wire_four'],
 'wire_five' => $data['wire_five'],
 'wire_six' => $data['wire_six']
);];
$db->insertFew("coils", array_keys($queryData), array_values($queryData));
public function submitCoil($data)
{
 ...
 if (!$this->validates($queryData['values'])) {
 return false;
 }
 return $db->insertFew("coils", $queryData['fields'], $queryData['values']);
}
protected function validates($values)
{
 $mandatoryFields = ['name', 'resistance', 'description', 'wraps'];
 $return = true;
 foreach($mandatoryFields as $field) {
 if (empty($values[$field]) {
 // Consider doing something like this so you can know/say
 // what the problem is.
 // $this->validationErrors[$field] = "This is empty";
 $return =false;
 }
 }
 return $return;
}
public function submitCoil($data)
{
 $queryData = $this->translateData($data);
 if (!$this->validates($queryData['values'])) {
 return false;
 }
 return $this->insert($queryData);
}
protected function translateData($data)
{
 $uniqueID = $this->hash("id");
 return array(
 'fields' => array(
 'uniqueid',
 'uploaded',
 'name',
 'resistance',
 'wrapped',
 'category',
 'description',
 'wraps',
 'images',
 'wire_one',
 'wire_two',
 'wire_three',
 'wire_four',
 'wire_five',
 'wire_six'
 ),
 'values' => array(
 'uniqueID' => $uniqueID,
 'uploaded' => date("F j, Y", time()),
 'name' => $data['name'],
 'resistance' => $data['resistance'],
 'wrapped' => $data['wrapped'],
 'category' => $data['category'],
 'description' => $data['description'],
 'wraps' => $data['wraps'],
 'images' => "coils/" . $uniqueID . "/",
 'wire_one' => $data['wire_one'],
 'wire_two' => $data['wire_two'],
 'wire_three' => $data['wire_three'],
 'wire_four' => $data['wire_four'],
 'wire_five' => $data['wire_five'],
 'wire_six' => $data['wire_six']
 )
 );
}
protected function validates($values)
{
 $mandatoryFields = ['name', 'resistance', 'description', 'wraps'];
 foreach($mandatoryFields as $field) {
 if (empty($values[$field]) {
 return false;
 }
 }
 return true;
}
protected function insert($data)
{
 $db = Database::getInstance();
 return $db->insertFew("coils", $data['fields'], $data['values']);
}
$queryData = array(
 'uniqueid' => $uniqueID,
 'uploaded' => date("F j, Y", time()),
 'name' => $data['name'],
 'resistance' => $data['resistance'],
 'wrapped' => $data['wrapped'],
 'category' => $data['category'],
 'description' => $data['description'],
 'wraps' => $data['wraps'],
 'images' => "coils/" . $uniqueID . "/",
 'wire_one' => $data['wire_one'],
 'wire_two' => $data['wire_two'],
 'wire_three' => $data['wire_three'],
 'wire_four' => $data['wire_four'],
 'wire_five' => $data['wire_five'],
 'wire_six' => $data['wire_six']
);
$db->insertFew("coils", array_keys($queryData), array_values($queryData));
public function submitCoil($data)
{
 ...
 if (!$this->validates($queryData['values'])) {
 return false;
 }
 return $db->insertFew("coils", $queryData['fields'], $queryData['values']);
}
protected function validates($values)
{
 $mandatoryFields = ['name', 'resistance', 'description', 'wraps'];
 $return = true;
 foreach($mandatoryFields as $field) {
 if (empty($values[$field])) {
 // Consider doing something like this so you can know/say
 // what the problem is.
 // $this->validationErrors[$field] = "This is empty";
 $return = false;
 }
 }
 return $return;
}
public function submitCoil($data)
{
 $queryData = $this->translateData($data);
 if (!$this->validates($queryData['values'])) {
 return false;
 }
 return $this->insert($queryData);
}
protected function translateData($data)
{
 $uniqueID = $this->hash("id");
 return [
 'fields' => [
 'uniqueid',
 'uploaded',
 'name',
 'resistance',
 'wrapped',
 'category',
 'description',
 'wraps',
 'images',
 'wire_one',
 'wire_two',
 'wire_three',
 'wire_four',
 'wire_five',
 'wire_six'
 ],
 'values' => [
 'uniqueID' => $uniqueID,
 'uploaded' => date("F j, Y", time()),
 'name' => $data['name'],
 'resistance' => $data['resistance'],
 'wrapped' => $data['wrapped'],
 'category' => $data['category'],
 'description' => $data['description'],
 'wraps' => $data['wraps'],
 'images' => "coils/" . $uniqueID . "/",
 'wire_one' => $data['wire_one'],
 'wire_two' => $data['wire_two'],
 'wire_three' => $data['wire_three'],
 'wire_four' => $data['wire_four'],
 'wire_five' => $data['wire_five'],
 'wire_six' => $data['wire_six']
 ]
 ];
}
protected function validates($values)
{
 $mandatoryFields = ['name', 'resistance', 'description', 'wraps'];
 foreach($mandatoryFields as $field) {
 if (empty($values[$field])) {
 return false;
 }
 }
 return true;
}
protected function insert($data)
{
 $db = Database::getInstance();
 return $db->insertFew("coils", $data['fields'], $data['values']);
}
$queryData = [
 'uniqueid' => $uniqueID,
 'uploaded' => date("F j, Y", time()),
 'name' => $data['name'],
 'resistance' => $data['resistance'],
 'wrapped' => $data['wrapped'],
 'category' => $data['category'],
 'description' => $data['description'],
 'wraps' => $data['wraps'],
 'images' => "coils/" . $uniqueID . "/",
 'wire_one' => $data['wire_one'],
 'wire_two' => $data['wire_two'],
 'wire_three' => $data['wire_three'],
 'wire_four' => $data['wire_four'],
 'wire_five' => $data['wire_five'],
 'wire_six' => $data['wire_six']
];
$db->insertFew("coils", array_keys($queryData), array_values($queryData));
added 137 characters in body
Source Link
AD7six
  • 1.4k
  • 9
  • 11

There are many possible solutions, whatone thing you can do is use a separate function to validate your data:

public function submitCoil ($data)
{
 ...
 if (!$this->validates($queryData['values'])) {
 return false;
 }
 return $db->insertFew("coils", $queryData['fields'], $queryData['values']);
}
protected function validates($values)
{
 $mandatoryFields = ['name', 'resistance', 'description', 'wraps'];
 $return = true;
 foreach($mandatoryFields as $field) {
 if (empty($values[$field]) {
 return// false;Consider doing something like this so you can know/say
 // what the problem is.
 // $this->validationErrors[$field] = "This is empty";
 $return =false;
  }
 }
 return true;$return;
}

This allows the building-blocks to be changed - or used individually. It also aides readability/maintainability as it is much clearer at a glance what a function does (assuming methods are well-named).

There are many possible solutions, what you can do is use a separate function to validate your data:

public function submitCoil ($data)
{
 ...
 if (!$this->validates($queryData['values'])) {
 return false;
 }
 return $db->insertFew("coils", $queryData['fields'], $queryData['values']);
}
protected function validates($values)
{
 $mandatoryFields = ['name', 'resistance', 'description', 'wraps'];
 foreach($mandatoryFields as $field) {
 if (empty($values[$field]) {
 return false;
 }
 }
 return true;
}

This allows the building-blocks to be changed - or used individually.

There are many possible solutions, one thing you can do is use a separate function to validate your data:

public function submitCoil ($data)
{
 ...
 if (!$this->validates($queryData['values'])) {
 return false;
 }
 return $db->insertFew("coils", $queryData['fields'], $queryData['values']);
}
protected function validates($values)
{
 $mandatoryFields = ['name', 'resistance', 'description', 'wraps'];
 $return = true;
 foreach($mandatoryFields as $field) {
 if (empty($values[$field]) {
 // Consider doing something like this so you can know/say
 // what the problem is.
 // $this->validationErrors[$field] = "This is empty";
 $return =false;
  }
 }
 return $return;
}

This allows the building-blocks to be changed - or used individually. It also aides readability/maintainability as it is much clearer at a glance what a function does (assuming methods are well-named).

Source Link
AD7six
  • 1.4k
  • 9
  • 11
Loading
lang-php

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