-1

I'm currently working on a codewars kata (Convert RGB number (0-255) to hexadecimal) and came across something interesting when trying to improve the readability of my code. I want to construct a global function to work with local variables, but when I try to run the code it throws an undefined error on the "j" variable (and also presumably the "i" variable). Code below:

function rgb(r, g, b){
  let x
  let y
  let z
  function conversion(){
    switch(j) {
            case 10:
              j = 'A'
              break
            case 11:
              j = 'B'
              break
            case 12:
              j = 'C'
              break
            case 13:
              j = 'D'
              break
            case 14:
              j = 'E'
              break
            case 15:
              j = 'F'
          }
          switch(i) {
            case 10:
              i = 'A'
              break
            case 11:
              i = 'B'
              break
            case 12:
              i = 'C'
              break
            case 13:
              i = 'D'
              break
            case 14:
              i = 'E'
              break
            case 15:
              i = 'F'
          }
  }
  if (r <= 0) {
    x = '00'
  } else if (r >= 255) {
      x = 'FF'
    } else {
        let i = Math.floor(r / 16)
        let j = Math.floor(r - i * 16)
        conversion()
        x = i.toString() + j.toString()
      }
  if (g <= 0) {
    y = '00'
  } else if (g >= 255) {
      y = 'FF'
    } else {
        let i = Math.floor(g / 16)
        let j = Math.floor(g - i * 16)
        conversion()
        y = i.toString() + j.toString()
      }
  if (b <= 0) {
    z = '00'
  } else if (b >= 255) {
      z = 'FF'
    } else {
        let i = Math.floor(b / 16)
        let j = Math.floor(b - i * 16)
        conversion()
        z = i.toString() + j.toString()
      }
  return x + y + z
}

So as you can see, it would be nicer to use the "conversion" function rather than copy paste all that code three times. Any ideas on this topic? Thanks in advance.

Heretic Monkey
  • 11,687
  • 7
  • 53
  • 122

3 Answers3

1

Variables declared with let and const are block-scoped. Your current setup doesn't work because i and j aren't declared within any of conversion's surrounding blocks.

While you could (begin to) fix it by declaring i and j in the outer function body:

function rgb(r, g, b){
  let x
  let y
  let z
  let j;
  let i;
  function conversion(){

It would be better to have conversion simply return the converted value, and assign the converted value to i or j. You can also make the rest of the code significantly less repetitive by making another function for converting the r, g, and b parameters, and then call that function 3 times, instead of copy-pasting the same sort of thing 3 times.

Also note that, to figure out the one's place, you can use just the modulo operator % on the number instead of having to subtract from the floored i.

function rgb(r, g, b) {
  // parameter should be 0-15
  function convertToSingleHexDigit(num) {
    return num <= 9
      ? String(num)
      : String.fromCharCode(55 + num); // 65 corresponds to A, 66 corresponds to B, etc
  }
  // parameter should be 0-255
  function convertNum(num) {
    const i = convertToSingleHexDigit(Math.floor(num / 16));
    const j = convertToSingleHexDigit(Math.floor(num % 16));
    return i + j;
  }
  return convertNum(r) + convertNum(g) + convertNum(b);
}
console.log(rgb(255, 255, 255));
console.log(rgb(100, 200, 100));
CertainPerformance
  • 356,069
  • 52
  • 309
  • 320
1

Since you declared i and j with let, their scope is just the else blocks where you declared them. Since conversion() is not defined inside those blocks, it can't access the variables.

You could declare the variables at the top of the function, and then assign them in the else blocks. Then they would be in the scope anything within the function, including the nested conversion() function.

But it would be better to just make them function parameters and return values.

function rgb(r, g, b){
  let x
  let y
  let z
  function conversion(){
    switch(j) {
            case 10:
              j = 'A'
              break
            case 11:
              j = 'B'
              break
            case 12:
              j = 'C'
              break
            case 13:
              j = 'D'
              break
            case 14:
              j = 'E'
              break
            case 15:
              j = 'F'
          }
          switch(i) {
            case 10:
              i = 'A'
              break
            case 11:
              i = 'B'
              break
            case 12:
              i = 'C'
              break
            case 13:
              i = 'D'
              break
            case 14:
              i = 'E'
              break
            case 15:
              i = 'F'
          }
    return [i, j]
  }
  if (r <= 0) {
    x = '00'
  } else if (r >= 255) {
      x = 'FF'
    } else {
        let i = Math.floor(r / 16)
        let j = Math.floor(r - i * 16)
        [i, j] = conversion(i, j)
        x = i.toString() + j.toString()
      }
  if (g <= 0) {
    y = '00'
  } else if (g >= 255) {
      y = 'FF'
    } else {
        let i = Math.floor(g / 16)
        let j = Math.floor(g - i * 16)
        [i, j] = conversion(i, j)
        y = i.toString() + j.toString()
      }
  if (b <= 0) {
    z = '00'
  } else if (b >= 255) {
      z = 'FF'
    } else {
        let i = Math.floor(b / 16)
        let j = Math.floor(b - i * 16)
        [i, j] = conversion(i, j)
        z = i.toString() + j.toString()
      }
  return x + y + z
}
Barmar
  • 741,623
  • 53
  • 500
  • 612
0

The only problem with your code that you have not declared j and i. Declare it either in outer function or inner function to make it work.

function rgb(r, g, b){
  let x
  let y
  let z
  // in outer function 
  let i;
  let j; 


  function conversion(){
    // in lkocal function here 
    // let j ;
    // let i; 

    switch(j) {
            case 10:
              j = 'A'
              break
            case 11:
              j = 'B'
              break
            case 12:
              j = 'C'
              break
            case 13:
              j = 'D'
              break
            case 14:
              j = 'E'
              break
            case 15:
              j = 'F'
          }
          switch(i) {
            case 10:
              i = 'A'
              break
            case 11:
              i = 'B'
              break
            case 12:
              i = 'C'
              break
            case 13:
              i = 'D'
              break
            case 14:
              i = 'E'
              break
            case 15:
              i = 'F'
          }
  }
  if (r <= 0) {
    x = '00'
  } else if (r >= 255) {
      x = 'FF'
    } else {
        let i = Math.floor(r / 16)
        let j = Math.floor(r - i * 16)
        conversion()
        x = i.toString() + j.toString()
      }
  if (g <= 0) {
    y = '00'
  } else if (g >= 255) {
      y = 'FF'
    } else {
        let i = Math.floor(g / 16)
        let j = Math.floor(g - i * 16)
        conversion()
        y = i.toString() + j.toString()
      }
  if (b <= 0) {
    z = '00'
  } else if (b >= 255) {
      z = 'FF'
    } else {
        let i = Math.floor(b / 16)
        let j = Math.floor(b - i * 16)
        conversion()
        z = i.toString() + j.toString()
      }
  return x + y + z
}


var r = rgb(56,67,90);

console.log(r);
debugmode
  • 936
  • 7
  • 13