1
\$\begingroup\$

The method that I wrote below functions properly; however, I believe that the code I wrote is not clean nor "good".

I know the basics of regex but I do not know more advanced techniques as I am still an amateur at programming.

static string SimpleColorOutput(Color color)
{
    //If input = Color[A=255, R=8, G=14, B=21]

    string colorText = color.ToString();

    Match R = Regex.Match(colorText, @"R=(\d+)[,\]]");
    Match G = Regex.Match(colorText, @"G=(\d+)[,\]]");
    Match B = Regex.Match(colorText, @"B=(\d+)[,\]]");

    return $"R = {R.Groups[1].Value}, G = {G.Groups[1].Value}, B = {B.Groups[1].Value}";

    //The method returns "R = 7, G = 13, B = 20"
}

Also, is it better to just pass in the Color object

\$\endgroup\$
  • 2
    \$\begingroup\$ What do you need this for? Can you give us more information? What you're doing is pointless as you could just use the color components without turning it into a s string first and then parsing it o_O \$\endgroup\$ – t3chb0t Aug 14 '19 at 7:43
  • 2
    \$\begingroup\$ How does R=8, G=14, B=21 turn into R = 7, G = 13, B = 20? Shouldn't all values be 1 higher? \$\endgroup\$ – Mast Aug 14 '19 at 12:32
  • \$\begingroup\$ @Mast apparently using regex doesn't cost a penny no longer applies; in fact it cost 3 pennies; ok, but at least the OP is consistent in his mistake ;-] \$\endgroup\$ – t3chb0t Aug 15 '19 at 7:00
4
\$\begingroup\$

The code in question is using the wrong tool (Regex) to achieve what you want. Using the string representation of a Color to get a string back is not a good way.

If one would pass a predefined color like Color.Red into that method it would fail by returning R = , G = , B = because the Color.ToString() returns

A string that is the name of this Color, if the Color is created from a predefined color by using either the FromName(String) method or the FromKnownColor(KnownColor) method; otherwise, a string that consists of the ARGB component names and their values.

If this is just for learning Regex you could change

string colorText = color.ToString();  

to

string colorText = Color.FromArgb(color.R, color.G, color.B).ToString();  

otherwise it would be better to just write

static string SimpleColorOutput(Color color)
{
    return $"R = {color.R}, G = {color.G}, B = {color.B}";
}
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thank you for your answer. The method was indeed just for Regex practice not for Color. I would love to know the correct way to use Regex here. \$\endgroup\$ – Shehab Ellithy Aug 15 '19 at 1:46
  • 1
    \$\begingroup\$ @ShehabEllithy For your next question, make sure you share that information clearly in the question before answers arrive. \$\endgroup\$ – Mast Aug 15 '19 at 9:28

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