Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am trying to convert hex to rgb, and rgb to hex. My current code doesn't seem optimal, and there is an issue with 0's in the hexadecimal. What do you think about the overall quality of this code? Can you think of a more efficient way of converting?

  $scope.$watch('hex', function() {
    var rgb = parseInt($scope.hex, 16);
    $scope.r = (rgb >> 16) & 0xFF;
    $scope.g = (rgb >> 8) & 0xFF;
    $scope.b = rgb & 0xFF;
    $scope.rgb = 'rgb(' + $scope.r + ',' + $scope.g + ',' + $scope.b + ');';
  });

  $scope.$watch('r+g+b', function() {
    $scope.rgb = 'rgb(' + $scope.r + ',' + $scope.g + ',' + $scope.b + ');';
    $scope.hex = parseInt($scope.r << 16 | $scope.g << 8 | $scope.b).toString(16);
  });

Here is a sample plunker:

share|improve this question

1 Answer 1

up vote 8 down vote accepted

A few notes:

  • I am confused as to why you need AngularJS for this.

  • I prefer to use the bitwise & with 255 instead of 0xFF in this case.

  • Your function can't handle non-hex characters such as #. I would remove them before parsing.

  • You can simplify your creation of the RGB string by using the join() method.

Final function:

function hexToRgb(hex) {
    hex = hex.replace(/[^0-9A-F]/gi, '');
    var bigint = parseInt(hex, 16);
    var r = (bigint >> 16) & 255;
    var g = (bigint >> 8) & 255;
    var b = bigint & 255;

    return [r, g, b].join();
}
share|improve this answer
    
I am using AngularJS because my application's UI will be easier to sync with Angular's 2 way data-binding. –  Josue Espinosa Aug 19 at 21:25
    
I totally disagree about using 255 instead of 0xFF here. I think 0xFF is preferred. –  Simon André Forsberg Oct 18 at 17:38

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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