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.

\$\endgroup\$
2
  • \$\begingroup\$ How is this different from your previous question? codereview.stackexchange.com/questions/52360/… \$\endgroup\$
    – cimmanon
    Commented Jun 15, 2014 at 20:31
  • \$\begingroup\$ @cimmanon I thought: this one is about the calculation. \$\endgroup\$
    – Bas
    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.

\$\endgroup\$

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.