Steganography program giving weird results

I am developing a steganography program for a computer programming class. It appears to gives random ascii symbols. The output is supposed to be BINARY. The encode message method was given to us by my teacher. We just have to program the decode part.

import java.awt.*;
class HideMessage {
    public void encodeMessage(Picture stegoObject, int[] binaryArray) {
        Pixel pixelTarget = new Pixel(stegoObject, 0, 0);
        Pixel[] pixelArray = stegoObject.getPixels();
        Color pixelColor = null;
        int redValue = 0;

        for (int x = 0; x < binaryArray.length; x++) {
            redValue = binaryArray[x];
            pixelTarget = pixelArray[x];
            pixelTarget.setRed(redValue);
        }
        pixelTarget = pixelArray[binaryArray.length];
        pixelTarget.setRed(255);
 开发者_开发百科       System.out.println("FinishedPic");
        stegoObject.write("SecretMessage.bmp");
        stegoObject.explore();

    }
    public void decodeMessage(Picture decodepic) {

        int redValue = 0;
        Pixel targetPixel = null;
        Color pixelColor = null;
        int sum = 0;

        for (int x = 0; redValue < 2; x++) {
                //inside nested loop to traverse the image from left to right
                for (int count = 1; count < 9; count++) {

                    targetPixel =
                        decodepic.getPixel(count + (8 * x), 0);
                    //gets the x,y coordinate of the target pixel
                    pixelColor = targetPixel.getColor();
                    //gets the color of the target pixel

                    redValue = pixelColor.getRed();

                    if (redValue == 1) {
                        if (count == 1) {
                            sum = sum + 128;
                        }
                        if (count == 2) {
                            sum = sum + 64;
                        }
                        if (count == 3) {
                            sum = sum + 32;
                        }
                        if (count == 4) {
                            sum = sum + 16;
                        }
                        if (count == 5) {
                            sum = sum + 8;
                        }
                        if (count == 6) {
                            sum = sum + 4;
                        }
                        if (count == 7) {
                            sum = sum + 2;
                        }
                        if (count == 8) {
                            sum = sum + 1;
                        }
                    }

                    System.out.println(sum);

                }
                System.out.println((char)sum);
                sum = 0;
            }   //end of the inner for loop
    }
}

public class HideMessageTester {
    public static void main(String[] args) {
        int[] bitArray =
            { 0, 1, 1, 0, 0, 0, 1, 0, 0, 1, 1, 0, 1, 0, 0, 1, 0, 1, 1,
           0, 1, 1, 1, 0, 0, 1, 1, 0, 0, 0, 0, 1, 0, 1, 1, 1, 0, 0, 1, 0, 0,
           1, 1, 1, 1, 0, 0, 1 };
        //int[] bitArray =
        { 0, 1, 0, 0, 1, 0, 0, 0, 0, 1, 1, 0, 1, 0, 0, 1, 0, 1, 1,
                0, 0, 1, 0, 0, 0, 1, 1, 0, 0, 1, 0, 0, 0, 1,
                1, 0, 0, 1, 0, 1, 0, 1, 1, 0, 1, 1, 1, 0, 0,
                0, 1, 0, 0, 0, 0, 0, 0, 1, 1, 0, 1, 0, 0, 1,
                0, 1, 1, 0, 1, 1, 1, 0, 0, 0, 1, 0, 0, 0, 0,
                0, 0, 1, 1, 1, 0, 0, 0, 0, 0, 1, 1, 0, 1, 1,
                0, 0, 0, 1, 1, 0, 0, 0, 0, 1, 0, 1, 1, 0, 1,
                0, 0, 1, 0, 1, 1, 0, 1, 1, 1, 0, 0, 0, 1, 0,
                0, 0, 0, 0, 0, 1, 1, 1, 0, 0, 1, 1, 0, 1, 1,
                0, 1, 0, 0, 1, 0, 1, 1, 0, 0, 1, 1, 1, 0, 1,
                1, 0, 1, 0, 0, 0, 0, 1, 1, 1, 0, 1, 0, 0, 0,
                0, 1, 0, 0, 0, 0, 1};

        Picture stegoObject = new Picture("Earth.bmp");

        HideMessage stego = new HideMessage();

        stego.encodeMessage(stegoObject, bitArray);
        Picture decodeObject = new Picture("SecretMessage.bmp");
        System.out.println("Now Decoding");
        stego.decodeMessage(decodeObject);
    }
}

First, some general pieces of advice: I think your program is overly complicated because the functions are commingling their responsibilities:

Picture stegoObject = new Picture("Earth.bmp");
HideMessage stego = new HideMessage();
stego.encodeMessage(stegoObject, bitArray);
Picture decodeObject = new Picture("SecretMessage.bmp");
System.out.println("Now Decoding");
stego.decodeMessage(decodeObject);

I was very surprised to see SecretMessage.bmp; it wasn't at all obvious that were trying to decode the object you had just created. Sure, upon reading the encodeMessage() method it was easy enough to determine where it came from, but I think this flow would have been easier:

/* encode */
Picture pic_to_steg = new Picture("foo.bmp");
HideMessage stego = new HideMessage();
Picture secret = stego.encodeMessage(pic_to_steg, bitArray);
secret.write("SecretMessage.bmp");

/* decode */
Picture pic_with_message = new Picture("SecretMessage.bmp");
int[] hidden = stego.decodeMessage(pic_with_message);

/* output `hidden` and compare against `bitArray` */

In other words: leave the file IO entirely up to the main flow of the program. Perhaps your routines will be called from a network server in the future, and the pictures will never be saved to disk. That modification will be far easier if the routines operate on Pictures and return amended Pictures and int[].

Can you test your encodeMessage() method in isolation? Perhaps look at the differences in what it does between an input file and an output file. This section looks troublesome:

public void encodeMessage(Picture stegoObject, int[] binaryArray) {
    Pixel pixelTarget = new Pixel(stegoObject, 0, 0);
    Pixel[] pixelArray = stegoObject.getPixels();
    Color pixelColor = null;
    int redValue = 0;

    for (int x = 0; x < binaryArray.length; x++) {
        redValue = binaryArray[x];
        pixelTarget = pixelArray[x];
        pixelTarget.setRed(redValue);
    }
    pixelTarget = pixelArray[binaryArray.length];
    pixelTarget.setRed(255);

Is the pixelArray really a reference into the image that can be updated through simple assignment? I'd really expect the design to look more like this pseudo-code:

pixel p = image.getPixel(x, y);
p.setred(binaryArray[i]);
image.setPixel(x, y, p);

The decoding has some strange loops:

    for (int x = 0; redValue < 2; x++) {
            //inside nested loop to traverse the image from left to right
            for (int count = 1; count < 9; count++) {

This loop might work exactly as you designed it, but upon a first reading, it feels very wrong: You start with x=0, you increment x each time through the loop, but you use redValue < 2 as your loop termination rule.

I would so much rather see the loop written like this:

int x = 0;
while (redValue < 2) {
    /* stuff */
    x++;
}

(It isn't identical; x is still valid outside the loop, which can be dangerous. However, I think this is much more clear.)

There are cases where the termination clause of a for loop isn't related to the setup or increment clauses -- in my experience, they are very rare.

In this case though, it feels like a mistake; the condition redValue < 2 a loop invariant, but the inner loop assumes it will only happen on pixels that are multiples of 8, which is an assumption that is not enforced in the encodeMessage() method.

Trying to compute an integer value from your redValues as you read them is needlessly complicating your decode routine. I suggest removing the inner loop and return an array exactly like the array passed into the encodeMessage() routine. This will be (a) easier (b) easier to debug (c) easier to test (d) a thousand times easier to handle writing bit arrays that aren't evenly divisible by 8.

Then write a second method that turns the bit array output into the sum, or ASCII characters, or EBCDIC characters, or RSA key parameters, or whatever it is that's being encoded. Don't try to do too much at once. Writing a separate method to decode the array of bits will be (a) easier (b) easier to debug (c) easier to test (d) thousand time easier to handle arbitrary output modifications.

I hope these hints help.