Steganography for images

For a school project I'm working on a small library to do steganography. I've just finished the first method of steganography and thought I'd get some feedback.

This steganography method works by changing the last bit of each channel of each pixel to a bit of the file. Reading the file from the image does the reverse operation.

The first point I'm unsure about, is my parameter and return types. The user will be able to hide any file in an image, so I'm currently asking for a Bitmap and a String with the file location. Should I ask for two file paths? Or should I ask for a Bitmap and a Path? I'm not sure. And what about the return types? Should I be giving back a byte[], or should I give back a file?

Besides that, general comments are welcome as well. Should anyone need it, the entire project is on GitHub.

```
public class SteganoBMP
{
//Embed a file in an image. Because Bitmap is an abstract representation of an image,
//it can then be saved in any image format. Though the file will not be retrievable from
//a lossy format.
public static Bitmap Embed(Bitmap target, String inputFilePath)
{
BitmapData bmpData = PrepareImage(target);

//Math.Abs because Stride can be negative if the image is saved upside down
//i.e. The first pixel in memory is the bottom right one.
int imageSize = Math.Abs(bmpData.Stride) * bmpData.Height;

//Get all the bytes from the file we want to embed, and save it in a byte array
byte[] fileBytes = File.ReadAllBytes(inputFilePath);

//If the file we want to embed is larger than 8 times the size of the image, we can't store it.
//This is because We need one byte in the image to store each bit of the file
if (fileBytes.Length * 8 > imageSize)
throw new FileTooLargeException("The file you are trying to embed needs an image of at least" + fileBytes.Length * 8 + "bytes lar

Good

• Most of the comments are telling why something is done.

• The input and local used parameter are named good

• The method- and parameternames fit the naming guidlines

Bad

• Some of the comments are useless, as they tell what is done like e.g

//Get all the bytes from the file we want to embed, and save it in a byte array
byte[] fileBytes = File.ReadAllBytes(inputFilePath); instead of why something is done.

• ifstatement without brackets {}

• The Extract() method does not call UnlockBits()

As you are unsure which inputparameters and which retuntypes to use, why don't you add some overloads of your methods?

public static Bitmap Embed(string imageFilePath, string inputFilePath)
{
    return InternalEmbed(new Bitmap(imageFilePath), File.ReadAllBytes(inputFilePath));
}
public static Bitmap Embed(Bitmap target, string inputFilePath)
{
    return InternalEmbed(target, File.ReadAllBytes(inputFilePath));
}
public static Bitmap Embed(Bitmap target, byte[] content)
{
    return InternalEmbed(target, content);
}

and rename your former public Embed() method to the private InternalEmbed() method

private  static Bitmap InternalEmbed(Bitmap target, byte[] fileBytes)
{
    // your former code without 
    // byte[] fileBytes = File.ReadAllBytes(inputFilePath);
}

After thinking about the overloads and looking at the signature of the methods I realized that if we return a Bitmap and also take a Bitmap as input parameter one would not assume that the passed Bitmap would be altered. As we have the overloads taking also a string we can't change the return type to void, therefor we need to call the InternalEmbed() method differently.

public static Bitmap Embed(Bitmap target, String inputFilePath)
{
    return InternalEmbed(new Bitmap(target), File.ReadAllBytes(inputFilePath));
}
public static Bitmap Embed(Bitmap target, byte[] content)
{
    return InternalEmbed(new Bitmap(target), content);
}

now we don't alter the passed Bitmap anymore.

StackExchange Code Review Q#63753, answer score: 9