Menu

#3 DEArgb needs some work

1.0
closed
None
2015-01-23
2015-01-17
No

int DEArgb( int AlphaValue, int RedValue, int GreenValue, int BlueValue ){
int RGBColor;
AlphaValue = ( AlphaValue & 255 ) * 16777216;
RedValue = ( RedValue & 255 ) * 65536;
GreenValue = ( GreenValue & 255 ) * 256;
BlueValue = BlueValue & 255 ;
RGBColor = ( AlphaValue + RedValue + GreenValue + BlueValue );
return RGBColor;
}

This function actually does work, but it is difficult to read for any programmer due to its use of unneeded local variables.

readability BUGlet:
normal convention when doing bitmasking is to use hexadecimal (or octal) and not decimal. Just because it is trivial to do bit <-> hex/octal conversions in your mind, while it is difficult to do in decimal.

A second readability BUG is RGBColor.
Why have a variable where the only use is to

  • assign to it
  • immediately return its value.
    Makes no sense and makes the code harder to read.
    Get rid of the variable and just return the value of the expression directly.

You probably want to rewrite this something like this instead:

int DEArgb( int AlphaValue, int RedValue, int GreenValue, int BlueValue ){
return (AlphaValue & 0xff) << 24 | (RedValue & 0xff) << 16
| (GreenValue & 0xff) << 8 | (BlueValue & 0xff);
}

Discussion

  • Cordier Frédéric ( AmiDARK )

    Hello,

    I understand your ideai but I found your version harder to understand as there are no description of what the function do...
    But I must admit that your version is more "optimised" than the original one.
    Changes will be done to this function.

    Thank you for your report.

     
  • Cordier Frédéric ( AmiDARK )

    • status: open --> closed
    • assigned_to: Cordier Frédéric ( AmiDARK )
     
  • Cordier Frédéric ( AmiDARK )

    Function improved and more readable.
    In 1 line I found it complex to read.I found it more clear in its new form.
    Similar to DEArgb way of working.

     

Log in to post a comment.

MongoDB Logo MongoDB