0
\$\begingroup\$

I am working a a C# program that will manipulate bitmap images. The first step is to read in the File Header and the Windows BITMAPINFOHEADER.

That will give me enough information to loop through pixels and do my manipulations.

Here is the class that reads the file header,

using System;
using System.IO;

namespace Bitmanipulate
{
    public class BMPFileHeader
    {
        char[] header_field = new char[2];
        private uint file_byte_size;
        private uint pixel_start;

        public BMPFileHeader(BinaryReader br)
        {
            br.BaseStream.Seek(0, SeekOrigin.Begin);
            header_field[0] = br.ReadChar();//1
            header_field[1] = br.ReadChar();//2
            file_byte_size = br.ReadUInt32();//6
            br.ReadUInt16();//8
            br.ReadUInt16();//10
            pixel_start = br.ReadUInt32();//14
        }

        public void show()
        {
            Console.WriteLine("Header Field");
            Console.WriteLine(header_field);
            Console.WriteLine("File size bytes");
            Console.WriteLine(file_byte_size);
            Console.WriteLine("Pixel Start");
            Console.WriteLine(pixel_start);
        }
    }
}

and here is the class reads the Windows BITMAPINFOHEADER

using System;
using System.IO;

namespace Bitmanipulate
{
    public class BMPHeaderReader
    {

        private uint header_size;
        private uint pixel_width;
        private uint pixel_height;
        private ushort pixel_depth;
        private uint row_size;
        private uint padding_bits;
        private uint padding_bytes;
        private uint width_bytes;
        private uint pixels;

        public BMPHeaderReader(BinaryReader br)
        {
            br.BaseStream.Seek(14, SeekOrigin.Begin);
            header_size = br.ReadUInt32();//18
            pixel_width = br.ReadUInt32();//22
            pixel_height = br.ReadUInt32();//26
            br.ReadUInt16();//28
            pixel_depth = br.ReadUInt16();//30
            row_size = (((pixel_depth * pixel_width) + 31) / 32) * 4;
            padding_bits = row_size * 8 - ((pixel_width * pixel_depth));
            padding_bytes = row_size - ((pixel_width * pixel_depth) / 8);
            width_bytes = (pixel_width * pixel_depth) / 8;
            pixels = pixel_height * pixel_width;
        }

        public void show()
        {
            Console.WriteLine("Header size");
            Console.WriteLine(header_size);
            Console.WriteLine("Pixel Width");
            Console.WriteLine(pixel_width);
            Console.WriteLine("Pixel Height");
            Console.WriteLine(pixel_height);
            Console.WriteLine("Pixel Depth");
            Console.WriteLine(pixel_depth);
            Console.WriteLine("Pixels");
            Console.WriteLine(pixels);
            Console.WriteLine("Row size");
            Console.WriteLine(row_size);
            Console.WriteLine("Padding bits");
            Console.WriteLine(padding_bits);
            Console.WriteLine("Padding bytes");
            Console.WriteLine(padding_bytes);

        }
    }
}

I have omitted the getters for clarity.

As this is basically my first ever C# program, is there anything that I am missing?

\$\endgroup\$
1
  • \$\begingroup\$ You say you have omitted getters for clarity, so I assume they look something like public uint HeaderSize { get { return header_size; } }? If so, you can clean things up by using auto-implemented properties: public uint HeaderSize { get; private set; } (and omitting the setter essentially gives you a 'readonly' property). \$\endgroup\$ Sep 15, 2017 at 7:09

1 Answer 1

5
\$\begingroup\$

First of all, don't use underscores in fields names. Fields like header_size, pixel_width and so on should be renamed to headerSize, pixelWidth. You should use camelCase for private fields, parameters and local variables. Also don't shorten parameters. Use binaryReader instead of br.

All methods in C# should be PascalCased. It means you should change show to Show.


I'm personally not a fan of passing streams and readers in constructors to initialize an instance of some class. In my opinion constructor should be as simple as possible, without reading from streams and heavy calculations. Ideally it should take only parameters that will be stored to fields or properties.

In you case I would change this code:

public BMPFileHeader(BinaryReader br)
{
    br.BaseStream.Seek(0, SeekOrigin.Begin);
    header_field[0] = br.ReadChar();//1
    header_field[1] = br.ReadChar();//2
    file_byte_size = br.ReadUInt32();//6
    br.ReadUInt16();//8
    br.ReadUInt16();//10
    pixel_start = br.ReadUInt32();//14
}

to

private const int HeaderFieldSize = 2;

private char[] _headerField = new char[HeaderFieldSize];
private uint _fileByteSize;
private uint _pixelStart;

public BMPFileHeader(char[] headerField, uint fileByteSize, uint pixelStart)
{
    _headerField = headerField;
    _fileByteSize = fileByteSize;
    _pixelStart = pixelStart;
}

public static BMPFileHeader Read(BinaryReader binaryReader)
{
    binaryReader.BaseStream.Seek(0, SeekOrigin.Begin);

    var headerField = binaryReader.ReadChars(HeaderFieldSize);
    var fileByteSize = binaryReader.ReadUInt32(); // 6

    binaryReader.ReadUInt16(); // 8
    binaryReader.ReadUInt16(); // 10

    var pixelStart = binaryReader.ReadUInt32(); // 14

    return new BMPFileHeader(headerField, fileByteSize, pixelStart);
}

And the same approach I would apply in BMPHeaderReader.


Instead of this code:

Console.WriteLine("Header size");
Console.WriteLine(header_size);
Console.WriteLine("Pixel Width");
Console.WriteLine(pixel_width);
Console.WriteLine("Pixel Height");
Console.WriteLine(pixel_height);
Console.WriteLine("Pixel Depth");
Console.WriteLine(pixel_depth);
Console.WriteLine("Pixels");
Console.WriteLine(pixels);
Console.WriteLine("Row size");
Console.WriteLine(row_size);
Console.WriteLine("Padding bits");
Console.WriteLine(padding_bits);
Console.WriteLine("Padding bytes");
Console.WriteLine(padding_bytes);

you can write

Console.WriteLine(string.Join(Environment.NewLine,
                              new object[]
                              {
                                  "Header size",   headerSize,
                                  "Pixel Width",   pixelWidth,
                                  "Pixel Height",  pixelHeight,
                                  "Pixel Depth",   pixelDepth,
                                  "Pixels",        pixels,
                                  "Row size",      rowSize,
                                  "Padding bits",  paddingBits,
                                  "Padding bytes", paddingBytes
                              });
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.