Skip to content

Conversation

Pr3d4dor
Copy link
Contributor

@Pr3d4dor Pr3d4dor commented May 2, 2020

  • Between rule for numeric types.

Closes #24

Copy link
Collaborator

@jasonmccreary jasonmccreary left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple small changes to tidy up.

array_push($rules, 'unique:' . $context . ',' . $column->name());
}

if (Str::contains($column->dataType(), 'decimal')) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These could all be refactored to in_array as the dataType will always been one of those.

@Pr3d4dor
Copy link
Contributor Author

Pr3d4dor commented May 2, 2020

@jasonmccreary I've already removed the default values, but in order to use in_array, the definition needs to be unsignedDecimal for example and unsigned decimal would not be possible in this way.

What do you want me to do?

@jasonmccreary
Copy link
Collaborator

@Pr3d4dor, maybe I've forgotten now, but I thought the root dataType was always decimal and the migration optimized the rest.

Either way, it seems to me you only need to know if the data type contains any of the numeric types and specifies a value. If so, then add the between rule.

My goal is always to write as little code as possible. Not one liners necessarily. But simple, readable code.

Give it one more review to see if you see anything. Then I will merge it.

@Pr3d4dor
Copy link
Contributor Author

Pr3d4dor commented May 2, 2020

@jasonmccreary Ok, I'll take a look and update the pull request if necessary.

@Pr3d4dor
Copy link
Contributor Author

Pr3d4dor commented May 2, 2020

DECIMAL, FLOAT and DOUBLE aren't the same thing:
https://dev.mysql.com/doc/refman/8.0/en/numeric-types.html

Laravel Code:

/**
 * Create a new float column on the table.
 *
 * @param  string  $column
 * @param  int  $total
 * @param  int  $places
 * @param  bool  $unsigned
 * @return \Illuminate\Database\Schema\ColumnDefinition
 */
public function float($column, $total = 8, $places = 2, $unsigned = false)
{
    return $this->addColumn('float', $column, compact('total', 'places', 'unsigned'));
}

/**
 * Create a new double column on the table.
 *
 * @param  string  $column
 * @param  int|null  $total
 * @param  int|null  $places
 * @param  bool  $unsigned
 * @return \Illuminate\Database\Schema\ColumnDefinition
 */
public function double($column, $total = null, $places = null, $unsigned = false)
{
    return $this->addColumn('double', $column, compact('total', 'places', 'unsigned'));
}

/**
 * Create a new decimal column on the table.
 *
 * @param  string  $column
 * @param  int  $total
 * @param  int  $places
 * @param  bool  $unsigned
 * @return \Illuminate\Database\Schema\ColumnDefinition
 */
public function decimal($column, $total = 8, $places = 2, $unsigned = false)
{
    return $this->addColumn('decimal', $column, compact('total', 'places', 'unsigned'));
}

@jasonmccreary
Copy link
Collaborator

jasonmccreary commented May 3, 2020

Ok. What's your point?

@Pr3d4dor
Copy link
Contributor Author

Pr3d4dor commented May 3, 2020

@jasonmccreary Just to verify if decimal was the root dataType or not like you said.

@jasonmccreary
Copy link
Collaborator

No worries. I'll finish this up. It'll be faster for me to just make the changes.

On a side note, I've noticed your last few PRs have a bunch of "merge master" commits. You can avoid this and have a cleaner PR by first pulling in the latest changes from Blueprint into your local master. Then create your feature branch.

@jasonmccreary jasonmccreary merged commit 0952428 into laravel-shift:master May 3, 2020
@Pr3d4dor Pr3d4dor deleted the feature/between-rule-for-numeric-types branch May 3, 2020 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add between rule for decimal or float columns
2 participants