[haiku-commits] Re: r34140 - haiku/trunk/src/apps/screenshot

Hi Fredrik,

hope you don't mind, but a few more things to point out with your patch:

> Modified: haiku/trunk/src/apps/screenshot/Screenshot.cpp 
> ===================================================================
> --- haiku/trunk/src/apps/screenshot/Screenshot.cpp    2009-11-19 16:33:52 
> UTC (rev 34139)
> +++ haiku/trunk/src/apps/screenshot/Screenshot.cpp    2009-11-19 20:47:17 
> UTC (rev 34140)
> @@ -1,10 +1,14 @@
>  /*
>   * Copyright Karsten Heimrich, host.haiku@xxxxxxx All rights reserved.
>   * Distributed under the terms of the MIT License.
> + *
> + * Authors:
> + *        Karsten Heimrich
> + *        Fredrik Modéen
>   */

To add yourself to a copyright like this, do this instead:

>   * Copyright Karsten Heimrich, host.haiku@xxxxxxx All rights reserved.
> + * Copyright Fredrik Modéen, ... All rights reserved.
>   * Distributed under the terms of the MIT License.
>   */

About the hard-coded fTranslator:

> +    fRefsReceived(false),
> +    fImageFileType(B_PNG_FORMAT),
> +    fTranslator(8)
>  {
>  }
>  

[...]

> +void
> +Screenshot::_SetImageTypeSilence(const char* name)
> +{
> +    if (strcmp(name, "bmp") == 0) {
> +        fImageFileType = B_BMP_FORMAT;
> +        fTranslator = 1;
> +    } else if (strcmp(name, "gif") == 0) {
> +        fImageFileType = B_GIF_FORMAT;
> +        fTranslator = 3;
> +    } else if (strcmp(name, "jpg") == 0) {
> +        fImageFileType = B_JPEG_FORMAT;
> +        fTranslator = 6;
> +    } else if (strcmp(name, "ppm") == 0) {
> +        fImageFileType = B_PPM_FORMAT;
> +        fTranslator = 9;
> +    } else if (strcmp(name, "targa") == 0) {
> +        fImageFileType = B_TGA_FORMAT;
> +        fTranslator = 14;
> +    } else if (strcmp(name, "tif") == 0) {
> +        fImageFileType = B_TIFF_FORMAT;
> +        fTranslator = 15;
> +    } else { //png
> +        fImageFileType = B_PNG_FORMAT;
> +        fTranslator = 8;
> +    }
> +}

Can you elaborate on that? It looks a bit like you have hard-coded values 
that depend on the runtime configuration of currently installed translators.

Best regards,
-Stephan

Other related posts: