KittyGiraudel/ama

Error handling in sass: am I doing it right?

Closed this issue · 6 comments

Hi,

I have a small question for you.
Currently I am building a very small utilities mixin library in sass to publish it later.
For handling common incorrect input argument on the mixins (numeric values and colors mostly) I did something like this:

// core of the handler (return error string if it find an error)
@function validate-input($numeric-values:null, $colors:null) {

  @each $numeric-value in $numeric-values {

    @if type-of($numeric-value) !=number and not index(initial inherit null auto, $numeric-value) {
      @return "The argument given as input must be a valid numeric-value, currently it is `#{$numeric-value}` (#{type-of($numeric-value)}), please check your input";
    }

  }

  @each $color in $colors {

    @if type-of($color) !=color and not index(initial inherit null auto, $color) {
      @return "The argument given as input must be a valid color, currently it is `#{$color}` (#{type-of($color)}), please check your input";
    }

  }

  @return false;
}

// simple mixin that act like a bridge between mixins and the function
@mixin validate-input($numeric-values:null, $colors:null) {

  $error-check: validate-input($numeric-values, $colors);

  @if $error-check {
    @error $error-check;
  }

}

// simple integration example 

@mixin line($width, $height, $color) {
// integrate the error handler
  @include validate-input($width $height, $color);

  content: "";
  display: block;
  @include size($width, $height);
  background-color: $color;
}

What do you thing about this method?
Is a to aggressive method? Should avoid it and do another simpler method?

Thank you

Hello!

I think it’s pretty good, although I would tackle it this way:

$ALLOWED_KEYWORDS: ('initial', 'inherit',  'auto', null);

@mixin ensure-type($type, $value) {
  @if type-of($value) != $type and not index($ALLOWED_KEYWORDS, $value) {
    @error "The argument given as input must be a valid #{$type}, currently it is `#{$value}` (#{type-of($value)}), please check your input";
  }
}

@mixin ensure-number($value) {
  @include ensure-type('number', $value);
}

@mixin ensure-color($value) {
  @include ensure-type('color', $value);
}

@mixin size($width, $height) {
  @include ensure-number($width);
  @include ensure-number($height);
  
  width: $width;
  height: $height;
}

@mixin line($width, $height, $color) {
  @include ensure-color($color);
  @include size($width, $height);
  content: "";
  display: block;
  background-color: $color;
}

.foo {
  @include line(10, 20px, 'red');
}

Hi, thank you very much for your
I think, that your implementation is plenty more flexible (you can check all the types you want) and well organized (without functions and necessary bridge).

The only downside about it, Is that you need for each value and type a @include directive, and it becomes uncomfortable when you have a lot of arguments passed or you need to refactor the mixins very frequently.

What do you think about this version of your implementation instead?

@mixin ensure-type($args...) {

  $allowed-keywords: ('initial', 'inherit', 'auto', null);
  $data-types: (number, string, color, list, map, bool, null);
  $type: null;

// take the type from the arguments
  @each $arg in $args {

    @if index($data-types, $arg) {
      $type: $arg;
    }

    @else {
 // core 
      @if type-of($arg) !=$type and not index($allowed-keywords, $arg) {
        @error "The argument given as input must be a `#{$type}`, currently it is `#{$arg}` with a data type of `#{type-of($arg)}`, please check your input";
      }
    }

  }
}
// example of integration

@mixin line($width, $height, $color) {
// Ensure that width and height are valid number and color is a valid color
  @include ensure-type(number, $width, $height, color, $color);

  content: "";
  display: block;
 width: $width;
height: $height;
  background-color: $color;
}


With this version you can check multiple types and values at once.

Little update:

//old
 $data-types: (number, string, color, list, map, bool, null);
//new
 $data-types: (number, string, color, list, map, bool);

Because if the user insert a null value like argument, the mixin will detect that the type to check is null.

I‘d recommend using a map instead of a list:

@include ensure-types((
  'number': ($width, $height),
  'color': $color
));

Oh, very good idea, it hadn't occurred to me.

thank you very much for all the help that you have given to me.
You can consider the issue closed!