6
\$\begingroup\$

I have a one function to calculate all the possible widths plus the margins for the gutters.

In this same function, the grid extensions such as the pull, pushand offset functions are also calculated.

@function calculate-one-column() {
 $a: 100%;
 @if ($gutter-width-px > 0px) {
 $a: ($a - ($gutter-width-procent * ($column-count - 1)));
 }
 @return $a / $column-count;
}
@function calculate-each-gutter() {
 @if ($gutter-width-px > 0px) {
 @return $gutter-width-procent;
 }
 @return 0;
}
@function calculate-function($function, $size, $first-child: false, $responsive: false) {
 $each-column: calculate-one-column() * $size;
 $each-gutter: calculate-each-gutter() * ($size - 1);
 @if ($function == 'size') {
 @return $each-column + $each-gutter;
 } @else if ($function == 'offset' and $first-child == false) and ($responsive == false) {
 @return $each-column + $each-gutter + ($gutter-width-procent * 1.5);
 } @else if ($function == 'push' or $function == 'pull') and ($responsive == false) or ($function == 'offset' and $first-child == true) {
 @return $each-column + $each-gutter + $gutter-width-procent;
 } @else if($function == 'offset' or 'push' or 'pull') and ($responsive == true) {
 @return auto !important;
 }
}

The calculate-function @function is getting called by this piece of code:

@mixin make-grid-sizes() {
 @for $index from 1 through $column-count {
 &.#{$column-size}#{$index} {
 @include calculate-property('width', $index);
 }
 }
}

All of these functions are getting hold together with these variables:

$column-count: 12;
$row-max-width: 1024px;
$gutter-width-px: 15px !default;
$gutter-width-procent: percentage($gutter-width-px / $row-max-width);

The code works perfectly, but in my opinion it looks very messy.

asked Jun 14, 2014 at 17:21
\$\endgroup\$
2
  • \$\begingroup\$ How is this different from your previous question? codereview.stackexchange.com/questions/52360/… \$\endgroup\$ Commented Jun 15, 2014 at 20:31
  • \$\begingroup\$ @cimmanon I thought: this one is about the calculation. \$\endgroup\$ Commented Jun 15, 2014 at 20:54

1 Answer 1

1
+50
\$\begingroup\$

Warning: I am far from an expert with Sass!

I'll list a few things I think could use some re-factoring.


We can DRY out your code. In the main function, I see $each-column + $each-gutter three times. Create a variable and assign the variable the sum.


This line: if ($function == 'offset' and $first-child == false) and ($responsive == false) can all be in one pair of parentheses. Breaking it apart actually reduces clarity.


I find that this line:

if ($function == 'push' or $function == 'pull') and ($responsive == false) or ($function == 'offset' and $first-child == true)

is difficult to read. Note my changes in the code below.


A couple instances, things seem mistaken. For example, this line:

if ($function == 'offset' or 'push' or 'pull') 

doesn't seem to be correct. I tried to copy this conditional, and I'm surprised you haven't noticed the error.

This statement says: If $function is equal to 'offset', or either 'push' is non-null or 'pull' is non-null. I believe it should be:

if ($function == 'offset' or $function == 'push' or $function == 'pull') and ($responsive == true) {

I've written your code to take into account all of these modifications. I think it looks neater, however it's not meant to be completely superficial. Since my Sass isn't very strong I hope someone else will be able to help break down your code with you more thoroughly.

@function calculate-function($function, $size, $first-child: false, $responsive: false) {
 $each-column: calculate-one-column() * $size;
 $each-gutter: calculate-each-gutter() * ($size - 1);
 $sum-width: $each-column + $each-gutter;
 $push-or-pull: ($function == 'push' or $function == 'pull');
 $offsetting: $function == 'offset';
 @if ($function == 'size') {
 @return $sum-width;
 } @else if ($offsetting and $first-child == false and $responsive == false) {
 @return $sum-width + ($gutter-width-percent * 1.5);
 } @else if $push-or-pull and ($responsive == false) or ($offsetting and $first-child) {
 @return $sum-width + $gutter-width-percent;
 } @else if ($offsetting or $push-or-pull) and $responsive {
 @return auto !important;
 }
}

Also, you misspelled percent as procent. I suggest you change that in all occurrences.

answered Jun 22, 2014 at 20:52
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.