2
\$\begingroup\$

I was testing the RGB color mechanic on Excel, so I did the following:

sub paint()
red = 255
blue = 0
green = 0
Max = 48
For col = 1 To Max
    For Row = 1 To Max
        Cells(Row, col).Interior.Color = RGB(red - col * Int(256 / (Max * 2)) - Row * Int(256 / (Max * 2)), green + col * Int(256 / Max), blue + Row * Int(256 / Max))
    Next Row
Next col
end sub

This makes an RBG chart on Excel filling the interior color of every cell on a max x max square. It was just made as a test and I think it worked pretty well.

I am looking for improvement, if possible.

\$\endgroup\$
2
\$\begingroup\$

The expressions Int(256 / (Max * 2)) and Int(256 / Max) have constant values inside the loop. It would be good to save them in a local variable before the loop and reuse.

The line where you set the cell color is too long, I have to scroll horizontally to see what's on the right hand side of the line. You could improve that by putting the red, green, blue values in variables. I would also rename the current red, green, blue variables to redBase, greenBase, blueBase, or similar.

The naming of the loop variables Row and col are inconsistent in capitalization. It would be better if they were either both lowercase or both starting with a capital letter. Similarly, it would be better to consistently capitalize all keywords, like you did For and Next.

\$\endgroup\$
  • \$\begingroup\$ I also didnt initialize any of the variables, thats why vba auto put Row and Max, but I get your point. I kinda dont understand the point of saving the Max variable twice, since both are tied together. If I change the number for a 10/10 square, I need the calculation to change based on 10. Line breaks on code is something I really need to work on. I tend to keep going and going, but I'll work on that. Thank you for your feedback! \$\endgroup\$ – Moacir Dec 6 '16 at 17:20
  • \$\begingroup\$ Int(256 / (Max * 2)) does a multiplication, a division, and integer truncation. The result is always the same, but in your code it's recalculated 48 * 48 times because it's inside the loop. It makes sense to extract it outside the loop. Another reason to do that is to give the expression a name, which could hint its purpose. \$\endgroup\$ – janos Dec 6 '16 at 17:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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