Skip to main content
added item about declarations
Source Link
200_success
  • 144.1k
  • 22
  • 188
  • 473

I think that changeColor or substColor would be a more appropriate name for this function.

I don't see why the function should write out its result to a file. That's a violation of the Single Responsibility Principle. What if I want to perform multiple color substitutions before writing out the result? What if I want a different output filename, or a format other than BMP?

Avoid empty declarations, when you can declare and initialize values at the same time. It's more readable and less error-prone.

You don't care about the x-y coordinates of each pixel — only the color matters. So, there is no need to construct the 2-D arrays representing the red, green, and blue channels.

/**
 * Changes all pixels of an old color into a new color, preserving the
 * alpha channel.
 */
private static void changeColor(
        BufferedImage imgBuf,
        int oldRed, int oldGreen, int oldBlue,
        int newRed, int newGreen, int newBlue) {

    int RGB_MASK = 0x00ffffff;
    int ALPHA_MASK = 0xff000000;

    int oldRGB = oldRed << 16 | oldGreen << 8 | oldBlue;
    int toggleRGB = oldRGB ^ (newRed << 16 | newGreen << 8 | newBlue);

    int w = imgBuf.getWidth();
    int h = imgBuf.getHeight();

    int[] rgb = imgBuf.getRGB(0, 0, w, h, null, 0, w);
    for (int i = 0; i < rgb.length; i++) {
        if ((rgb[i] & RGB_MASK) == oldRGB) {
            rgb[i] ^= toggleRGB;
        }
    }
    imgBuf.setRGB(0, 0, w, h, rgb, 0, w);
}

public static void main(String[] args) throws IOException {
    BufferedImage pic1 = ImageIO.read(new File(…));
    changeColor(pic1, 34, 177, 76, 127, 127, 127);
    ImageIO.write(pic1, "bmp", new File(…));
}

I think that changeColor or substColor would be a more appropriate name for this function.

I don't see why the function should write out its result to a file. That's a violation of the Single Responsibility Principle. What if I want to perform multiple color substitutions before writing out the result? What if I want a different output filename, or a format other than BMP?

You don't care about the x-y coordinates of each pixel — only the color matters. So, there is no need to construct the 2-D arrays representing the red, green, and blue channels.

/**
 * Changes all pixels of an old color into a new color, preserving the
 * alpha channel.
 */
private static void changeColor(
        BufferedImage imgBuf,
        int oldRed, int oldGreen, int oldBlue,
        int newRed, int newGreen, int newBlue) {

    int RGB_MASK = 0x00ffffff;
    int ALPHA_MASK = 0xff000000;

    int oldRGB = oldRed << 16 | oldGreen << 8 | oldBlue;
    int toggleRGB = oldRGB ^ (newRed << 16 | newGreen << 8 | newBlue);

    int w = imgBuf.getWidth();
    int h = imgBuf.getHeight();

    int[] rgb = imgBuf.getRGB(0, 0, w, h, null, 0, w);
    for (int i = 0; i < rgb.length; i++) {
        if ((rgb[i] & RGB_MASK) == oldRGB) {
            rgb[i] ^= toggleRGB;
        }
    }
    imgBuf.setRGB(0, 0, w, h, rgb, 0, w);
}

public static void main(String[] args) throws IOException {
    BufferedImage pic1 = ImageIO.read(new File(…));
    changeColor(pic1, 34, 177, 76, 127, 127, 127);
    ImageIO.write(pic1, "bmp", new File(…));
}

I think that changeColor or substColor would be a more appropriate name for this function.

I don't see why the function should write out its result to a file. That's a violation of the Single Responsibility Principle. What if I want to perform multiple color substitutions before writing out the result? What if I want a different output filename, or a format other than BMP?

Avoid empty declarations, when you can declare and initialize values at the same time. It's more readable and less error-prone.

You don't care about the x-y coordinates of each pixel — only the color matters. So, there is no need to construct the 2-D arrays representing the red, green, and blue channels.

/**
 * Changes all pixels of an old color into a new color, preserving the
 * alpha channel.
 */
private static void changeColor(
        BufferedImage imgBuf,
        int oldRed, int oldGreen, int oldBlue,
        int newRed, int newGreen, int newBlue) {

    int RGB_MASK = 0x00ffffff;
    int ALPHA_MASK = 0xff000000;

    int oldRGB = oldRed << 16 | oldGreen << 8 | oldBlue;
    int toggleRGB = oldRGB ^ (newRed << 16 | newGreen << 8 | newBlue);

    int w = imgBuf.getWidth();
    int h = imgBuf.getHeight();

    int[] rgb = imgBuf.getRGB(0, 0, w, h, null, 0, w);
    for (int i = 0; i < rgb.length; i++) {
        if ((rgb[i] & RGB_MASK) == oldRGB) {
            rgb[i] ^= toggleRGB;
        }
    }
    imgBuf.setRGB(0, 0, w, h, rgb, 0, w);
}

public static void main(String[] args) throws IOException {
    BufferedImage pic1 = ImageIO.read(new File(…));
    changeColor(pic1, 34, 177, 76, 127, 127, 127);
    ImageIO.write(pic1, "bmp", new File(…));
}
added 73 characters in body
Source Link
200_success
  • 144.1k
  • 22
  • 188
  • 473

I think that changeColor or substColor would be a more appropriate name for this function.

I don't see why itthe function should write out its result to a file. That's a violation of the Single Responsibility Principle. What if I want to perform multiple color substitutions before writing out the result? What if I want a different output filename, or a format other than BMP?

You don't care about the x-y coordinates of each pixel — only the color matters. So, there is no need to construct the 2-D arrays representing the red, green, and blue channels.

/**
 * Changes all pixels of an old color into a new color, preserving the
 * alpha channel.
 */
private static void changeColor(
        BufferedImage imgBuf,
        int oldRed, int oldGreen, int oldBlue,
        int newRed, int newGreen, int newBlue) {

    int RGB_MASK = 0x00ffffff;
    int ALPHA_MASK = 0xff000000;

    int oldRGB = oldRed << 16 | oldGreen << 8 | oldBlue;
    int toggleRGB = oldRGB ^ (newRed << 16 | newGreen << 8 | newBlue);

    int w = imgBuf.getWidth();
    int h = imgBuf.getHeight();

    int[] rgb = imgBuf.getRGB(0, 0, w, h, null, 0, w);
    for (int i = 0; i < rgb.length; i++) {
        if ((rgb[i] & RGB_MASK) == oldRGB) {
            rgb[i] ^= toggleRGB;
        }
    }
    imgBuf.setRGB(0, 0, w, h, rgb, 0, w);
}

public static void main(String[] args) throws IOException {
    BufferedImage pic1 = ImageIO.read(new File(…));
    changeColor(pic1, 34, 177, 76, 127, 127, 127);
    ImageIO.write(pic1, "bmp", new File(…));
}

I think that changeColor or substColor would be a more appropriate name for this function.

I don't see why it should write out its result to a file. That's a violation of the Single Responsibility Principle. What if I want to perform multiple color substitutions before writing out the result?

You don't care about the x-y coordinates of each pixel — only the color matters. So, there is no need to construct the 2-D arrays representing the red, green, and blue channels.

/**
 * Changes all pixels of an old color into a new color, preserving the
 * alpha channel.
 */
private static void changeColor(
        BufferedImage imgBuf,
        int oldRed, int oldGreen, int oldBlue,
        int newRed, int newGreen, int newBlue) {

    int RGB_MASK = 0x00ffffff;
    int ALPHA_MASK = 0xff000000;

    int oldRGB = oldRed << 16 | oldGreen << 8 | oldBlue;
    int toggleRGB = oldRGB ^ (newRed << 16 | newGreen << 8 | newBlue);

    int w = imgBuf.getWidth();
    int h = imgBuf.getHeight();

    int[] rgb = imgBuf.getRGB(0, 0, w, h, null, 0, w);
    for (int i = 0; i < rgb.length; i++) {
        if ((rgb[i] & RGB_MASK) == oldRGB) {
            rgb[i] ^= toggleRGB;
        }
    }
    imgBuf.setRGB(0, 0, w, h, rgb, 0, w);
}

public static void main(String[] args) throws IOException {
    BufferedImage pic1 = ImageIO.read(new File(…));
    changeColor(pic1, 34, 177, 76, 127, 127, 127);
    ImageIO.write(pic1, "bmp", new File(…));
}

I think that changeColor or substColor would be a more appropriate name for this function.

I don't see why the function should write out its result to a file. That's a violation of the Single Responsibility Principle. What if I want to perform multiple color substitutions before writing out the result? What if I want a different output filename, or a format other than BMP?

You don't care about the x-y coordinates of each pixel — only the color matters. So, there is no need to construct the 2-D arrays representing the red, green, and blue channels.

/**
 * Changes all pixels of an old color into a new color, preserving the
 * alpha channel.
 */
private static void changeColor(
        BufferedImage imgBuf,
        int oldRed, int oldGreen, int oldBlue,
        int newRed, int newGreen, int newBlue) {

    int RGB_MASK = 0x00ffffff;
    int ALPHA_MASK = 0xff000000;

    int oldRGB = oldRed << 16 | oldGreen << 8 | oldBlue;
    int toggleRGB = oldRGB ^ (newRed << 16 | newGreen << 8 | newBlue);

    int w = imgBuf.getWidth();
    int h = imgBuf.getHeight();

    int[] rgb = imgBuf.getRGB(0, 0, w, h, null, 0, w);
    for (int i = 0; i < rgb.length; i++) {
        if ((rgb[i] & RGB_MASK) == oldRGB) {
            rgb[i] ^= toggleRGB;
        }
    }
    imgBuf.setRGB(0, 0, w, h, rgb, 0, w);
}

public static void main(String[] args) throws IOException {
    BufferedImage pic1 = ImageIO.read(new File(…));
    changeColor(pic1, 34, 177, 76, 127, 127, 127);
    ImageIO.write(pic1, "bmp", new File(…));
}
Source Link
200_success
  • 144.1k
  • 22
  • 188
  • 473

I think that changeColor or substColor would be a more appropriate name for this function.

I don't see why it should write out its result to a file. That's a violation of the Single Responsibility Principle. What if I want to perform multiple color substitutions before writing out the result?

You don't care about the x-y coordinates of each pixel — only the color matters. So, there is no need to construct the 2-D arrays representing the red, green, and blue channels.

/**
 * Changes all pixels of an old color into a new color, preserving the
 * alpha channel.
 */
private static void changeColor(
        BufferedImage imgBuf,
        int oldRed, int oldGreen, int oldBlue,
        int newRed, int newGreen, int newBlue) {

    int RGB_MASK = 0x00ffffff;
    int ALPHA_MASK = 0xff000000;

    int oldRGB = oldRed << 16 | oldGreen << 8 | oldBlue;
    int toggleRGB = oldRGB ^ (newRed << 16 | newGreen << 8 | newBlue);

    int w = imgBuf.getWidth();
    int h = imgBuf.getHeight();

    int[] rgb = imgBuf.getRGB(0, 0, w, h, null, 0, w);
    for (int i = 0; i < rgb.length; i++) {
        if ((rgb[i] & RGB_MASK) == oldRGB) {
            rgb[i] ^= toggleRGB;
        }
    }
    imgBuf.setRGB(0, 0, w, h, rgb, 0, w);
}

public static void main(String[] args) throws IOException {
    BufferedImage pic1 = ImageIO.read(new File(…));
    changeColor(pic1, 34, 177, 76, 127, 127, 127);
    ImageIO.write(pic1, "bmp", new File(…));
}