7
\$\begingroup\$

So my this class is one in which is used to find rectangles in images that have already been pre-processed. For example the images are to be deskewed, greyscale etc. Following feedback off of various people I have been told to try and conform it to c# coding standards. I have tried to do my best but was wondering what you guys thought. The code is below:

using System;
using System.Collections.Generic;
using System.Drawing;
using System.Drawing.Imaging;
using System.Linq;

namespace Recognition
{
    public class OMRRecognition : IDisposable
    {
        private const int BYTES_PER_PIXEL = 3;

        private const int MAX_BLACK_VALUE = 382; // (255 * 3) / 2 rounded down

        private const float MIN_ZONE_MULTIPLIER = 0.85f;
        private const float MAX_ZONE_MULTIPLIER = 1.15f;

        private const float LINE_BROKEN_TOLERANCE = 0.10f;

        private bool _bImageOpen;

        private int _nBitmapWidth;
        private int _nBitmapHeight;

        private int _nStride;
        private byte[] _baPixels;

        private int _nCornerTolerance;
        private int _nMaxHorBrokenAmount;
        private int _nMaxVertBrokenAmount;
        private Size _sExpectedZoneSize;
        private Size _sMinZoneSize;
        private Size _sMaxZoneSize;

        public OMRRecognition()
        {
            SetZoneSize(new Size(10, 10));
            SetCornerTolerance(5);

            _bImageOpen = false;
        }

        public void Dispose()
        {
            _baPixels = null;

            _bImageOpen = false;
        }

        public void SetZoneSize(Size zoneSize)
        {
            _sExpectedZoneSize = zoneSize;
            _sMinZoneSize = new Size((int)(zoneSize.Width * MIN_ZONE_MULTIPLIER), (int)(zoneSize.Height * MIN_ZONE_MULTIPLIER));
            _sMaxZoneSize = new Size((int)(zoneSize.Width * MAX_ZONE_MULTIPLIER), (int)(zoneSize.Height * MAX_ZONE_MULTIPLIER));

            _nMaxHorBrokenAmount = (int)Math.Ceiling(zoneSize.Width * LINE_BROKEN_TOLERANCE);
            _nMaxVertBrokenAmount = (int)Math.Ceiling(zoneSize.Height * LINE_BROKEN_TOLERANCE);
        }

        public void SetCornerTolerance(int cornerTolerance)
        {
            _nCornerTolerance = cornerTolerance;
        }

        public void OpenImage(Bitmap bitmap)
        {
            using (Bitmap tempBitmap = new Bitmap(bitmap.Width, bitmap.Height, PixelFormat.Format24bppRgb))
            {
                using (Graphics g = Graphics.FromImage(tempBitmap))
                {
                    g.DrawImage(bitmap, new Rectangle(0, 0, tempBitmap.Width, tempBitmap.Height));
                }

                _nBitmapWidth = tempBitmap.Width;
                _nBitmapHeight = tempBitmap.Height;

                BitmapData bitmapData = tempBitmap.LockBits(new Rectangle(0, 0, _nBitmapWidth, _nBitmapHeight), ImageLockMode.ReadOnly, tempBitmap.PixelFormat);
                try
                {
                    int nByteCount = bitmapData.Stride * _nBitmapHeight;
                    _baPixels = new byte[nByteCount];

                    System.Runtime.InteropServices.Marshal.Copy(bitmapData.Scan0, _baPixels, 0, _baPixels.Length);
                    _nStride = bitmapData.Stride;
                }
                finally
                {
                    tempBitmap.UnlockBits(bitmapData);
                }
            }

            _bImageOpen = true;
        }

        public OMRZone[] PerformRecognition()
        {
            try
            {
                if (!_bImageOpen)
                    throw new Exception("An image must be opened first using the 'OpenImage' method!");

                return ExtractZones();
            }
            catch (Exception ex)
            {
                return null;
            }
        }

        private OMRZone[] ExtractZones()
        {
            Line[] laHorizontalLines = FindHorizontalLines();
            if (laHorizontalLines.Length < 1)
                throw new Exception("No horizontal lines were found!");

            Line[] laVerticalLines = FindVerticalLines();
            if (laVerticalLines.Length < 1)
                throw new Exception("No vertical lines were found!");

            List<Pixel[]> palAllCorners = FindCorners(laHorizontalLines, laVerticalLines);

            return FindRectangles(palAllCorners);
        }

        private Line[] FindHorizontalLines()
        {
            int nZoneRight = _nBitmapWidth * BYTES_PER_PIXEL;

            List<Line> llHorizontalLines = new List<Line>();
            for (int h = 0; h < _nBitmapHeight; h++)
            {
                int nCurrentLine = h * _nStride;
                int nBrokenAmount = 0;
                bool bFirstPixelFound = false;
                List<Pixel> horizontalLine = new List<Pixel>();

                for (int w = 0; w < nZoneRight; w += BYTES_PER_PIXEL)
                {
                    int nBlue = _baPixels[nCurrentLine + w];
                    int nGreen = _baPixels[nCurrentLine + w + 1];
                    int nRed = _baPixels[nCurrentLine + w + 2];

                    if (nBlue + nGreen + nRed <= MAX_BLACK_VALUE)
                    {
                        nBrokenAmount = 0;
                        bFirstPixelFound = true;
                        horizontalLine.Add(new Pixel((w / BYTES_PER_PIXEL), h, true, 0));
                    }
                    else
                    {
                        if (!bFirstPixelFound)
                            continue;

                        nBrokenAmount += 1;
                        float fBrokenConfidence = 0f;

                        if (nBrokenAmount <= _nMaxHorBrokenAmount)
                        {
                            horizontalLine.Add(new Pixel((w / BYTES_PER_PIXEL), h, false, 0));
                            continue;
                        }
                        else if (horizontalLine.FirstOrDefault(p => p.IsBlack) == null)
                        {
                            //If line is all white clear vars and continue
                            nBrokenAmount = 0;
                            horizontalLine.Clear();
                            bFirstPixelFound = false;
                            continue;
                        }
                        else
                        {
                            //Remove white pixels from end of line
                            for (int i = horizontalLine.Count - 1; i > 0; i--)
                            {
                                if (!horizontalLine[i].IsBlack)
                                    horizontalLine.RemoveAt(i);

                                else
                                    break;
                            }

                            int nTotalBrokenAmount = horizontalLine.Count(p => !p.IsBlack);

                            fBrokenConfidence = 100f - (((float)nTotalBrokenAmount / (float)horizontalLine.Count) * 100f);
                            nBrokenAmount = 0;
                            bFirstPixelFound = false;
                        }

                        if (horizontalLine.Count < _sMinZoneSize.Width || horizontalLine.Count > _sMaxZoneSize.Width)
                        {
                            nBrokenAmount = 0;
                            horizontalLine.Clear();
                            bFirstPixelFound = false;
                            continue;
                        }

                        horizontalLine = horizontalLine.OrderBy(p => p.X).ToList();
                        llHorizontalLines.Add(new Line(horizontalLine[0].ToPoint(), horizontalLine[horizontalLine.Count - 1].ToPoint(), fBrokenConfidence));
                        horizontalLine.Clear();
                    }
                }
            }

            return llHorizontalLines.ToArray();
        }

        private Line[] FindVerticalLines()
        {
            int nZoneRight = _nBitmapWidth * BYTES_PER_PIXEL;

            List<Line> llVerticalLines = new List<Line>();
            for (int w = 0; w < nZoneRight; w += BYTES_PER_PIXEL)
            {
                List<Pixel> verticalLine = new List<Pixel>();
                int nBrokenAmount = 0;
                bool bFirstPixelFound = false;

                for (int h = 0; h < _nBitmapHeight; h++)
                {
                    int nCurrentLine = h * _nStride;

                    int nBlue = _baPixels[nCurrentLine + w];
                    int nGreen = _baPixels[nCurrentLine + w + 1];
                    int nRed = _baPixels[nCurrentLine + w + 2];

                    if (nBlue + nGreen + nRed <= MAX_BLACK_VALUE)
                    {
                        nBrokenAmount = 0;
                        bFirstPixelFound = true;
                        verticalLine.Add(new Pixel((w / BYTES_PER_PIXEL), h, true, 0));
                    }
                    else
                    {
                        if (!bFirstPixelFound)
                            continue;

                        nBrokenAmount += 1;
                        float fBrokenConfidence = 0f;

                        if (nBrokenAmount <= _nMaxVertBrokenAmount)
                        {
                            verticalLine.Add(new Pixel((w / BYTES_PER_PIXEL), h, false, 0));
                            continue;
                        }
                        else if (verticalLine.FirstOrDefault(p => p.IsBlack) == null)
                        {
                            //If line is all white clear vars and continue
                            nBrokenAmount = 0;
                            verticalLine.Clear();
                            bFirstPixelFound = false;
                            continue;
                        }
                        else
                        {
                            //Remove white pixels from end of line
                            for (int i = verticalLine.Count - 1; i > 0; i--)
                            {
                                if (!verticalLine[i].IsBlack)
                                    verticalLine.RemoveAt(i);

                                else
                                    break;
                            }

                            int nTotalBrokenAmount = verticalLine.Count(p => !p.IsBlack);

                            fBrokenConfidence = 100f - (((float)nTotalBrokenAmount / (float)verticalLine.Count) * 100f);
                            nBrokenAmount = 0;
                            bFirstPixelFound = false;
                        }

                        if (verticalLine.Count < _sMinZoneSize.Height || verticalLine.Count > _sMaxZoneSize.Height)
                        {
                            nBrokenAmount = 0;
                            verticalLine.Clear();
                            bFirstPixelFound = false;
                            continue;
                        }

                        verticalLine = verticalLine.OrderBy(p => p.Y).ToList();
                        llVerticalLines.Add(new Line(verticalLine[0].ToPoint(), verticalLine[verticalLine.Count - 1].ToPoint(), fBrokenConfidence));
                        verticalLine.Clear();
                    }
                }
            }

            return llVerticalLines.ToArray();
        }

        private List<Pixel[]> FindCorners(Line[] horizontalLines, Line[] verticalLines)
        {
            Pixel[] paTopLeftCorners = FindTopLeftCorners(horizontalLines, verticalLines);
            Pixel[] paTopRightCorners = FindTopRightCorners(horizontalLines, verticalLines);
            Pixel[] paBottomLeftCorners = FindBottomLeftCorners(horizontalLines, verticalLines);
            Pixel[] paBottomRightCorners = FindBottomRightCorners(horizontalLines, verticalLines);

            Pixel[] paAverageTopLeftCorners = paTopLeftCorners.RemoveNearbyPixels(_nCornerTolerance, true, true);
            Pixel[] paAverageTopRightCorners = paTopRightCorners.RemoveNearbyPixels(_nCornerTolerance, true, false);
            Pixel[] paAverageBottomLeftCorners = paBottomLeftCorners.RemoveNearbyPixels(_nCornerTolerance, false, true);
            Pixel[] paAverageBottomRightCorners = paBottomRightCorners.RemoveNearbyPixels(_nCornerTolerance, false, false);

            return new List<Pixel[]>() { paAverageTopLeftCorners, paAverageTopRightCorners, paAverageBottomLeftCorners, paAverageBottomRightCorners };
        }

        private Pixel[] FindTopLeftCorners(Line[] horizontalLines, Line[] verticalLines)
        {
            List<Pixel> TopLeftCorners = new List<Pixel>();

            Line[] laHorizontalLines = horizontalLines.OrderBy(l => l.StartPoint.X).ThenBy(l => l.StartPoint.Y).ToArray();
            Line[] laVerticalLines = verticalLines.OrderBy(l => l.StartPoint.X).ThenBy(l => l.StartPoint.Y).ToArray();

            foreach (Line verticalLine in laVerticalLines)
            {
                foreach (Line horizontalLine in laHorizontalLines)
                {
                    if (verticalLine.StartPoint.X <= (horizontalLine.StartPoint.X + _nCornerTolerance) && verticalLine.StartPoint.X >= (horizontalLine.StartPoint.X - _nCornerTolerance))
                    {
                        if (horizontalLine.StartPoint.Y <= (verticalLine.StartPoint.Y + _nCornerTolerance) && horizontalLine.StartPoint.Y >= (verticalLine.StartPoint.Y - _nCornerTolerance))
                        {
                            float fCornerConfidence = 100f - (((float)Math.Abs(verticalLine.StartPoint.X - horizontalLine.StartPoint.X) + (float)Math.Abs(verticalLine.StartPoint.Y - horizontalLine.StartPoint.Y)) * 5f);
                            float fConfidence = (((verticalLine.Confidence + horizontalLine.Confidence) * 2f) + fCornerConfidence) / 5f;

                            int nX = Math.Abs((verticalLine.StartPoint.X + horizontalLine.StartPoint.X) / 2);
                            int nY = Math.Abs((verticalLine.StartPoint.Y + horizontalLine.StartPoint.Y) / 2);

                            TopLeftCorners.Add(new Pixel(nX, nY, true, fConfidence));
                            break;
                        }
                    }
                }
            }

            return TopLeftCorners.Distinct().ToArray();
        }

        //The other `FindCorner` Methods are in the same format as above

        private OMRZone[] FindRectangles(List<Pixel[]> corners)
        {
            Pixel[] paAverageTopLeftCorners = corners[0].Distinct().ToArray();
            Pixel[] paAverageTopRightCorners = corners[1].OrderBy(c => c.X).Distinct().ToArray();
            Pixel[] paAverageBottomLeftCorners = corners[2].OrderBy(c => c.Y).Distinct().ToArray();
            Pixel[] paAverageBottomRightCorners = corners[3].OrderBy(c => c.X).ThenBy(c => c.Y).Distinct().ToArray();

            List<OMRZone> zlZones = new List<OMRZone>();
            foreach (Pixel pTopLeft in paAverageTopLeftCorners)
            {
                try
                {
                    Pixel[] paTempTRC = paAverageTopRightCorners.Select(p => p).Where(p => p.Y >= (pTopLeft.Y - _nCornerTolerance) && p.Y <= (pTopLeft.Y + _nCornerTolerance) && (p.X - pTopLeft.X) >= _sMinZoneSize.Width && (p.X - pTopLeft.X) <= _sMaxZoneSize.Width).ToArray();
                    if (paTempTRC.Length > 0)
                    {
                        Pixel pTopRight = paTempTRC.RemoveNearbyPixels(_nCornerTolerance, true, false).FirstOrDefault();
                        float fTopRightConfidence = 100f - ((float)Math.Abs(pTopLeft.Y - pTopRight.Y) * 5f);

                        Pixel[] paTempBLC = paAverageBottomLeftCorners.Select(p => p).Where(p => p.X >= (pTopLeft.X - _nCornerTolerance) && p.X <= (pTopLeft.X + _nCornerTolerance) && (p.Y - pTopLeft.Y) >= _sMinZoneSize.Height && (p.Y - pTopLeft.Y) <= _sMaxZoneSize.Height).ToArray();
                        if (paTempBLC.Length > 0)
                        {
                            Pixel pBottomLeft = paTempBLC.RemoveNearbyPixels(_nCornerTolerance, false, true).FirstOrDefault();
                            float fBottomLeftConfidence = 100f - ((float)Math.Abs(pTopLeft.X - pBottomLeft.X) * 5f);

                            Pixel[] paTempBRC = paAverageBottomRightCorners.Select(c => c).Where(c => c.X > pBottomLeft.X && c.Y > pTopRight.Y).ToArray();
                            Pixel pBottomRight = paTempBRC.FirstOrDefault(p => p.Y >= (pBottomLeft.Y - _nCornerTolerance) && p.Y <= (pBottomLeft.Y + _nCornerTolerance) && p.X >= (pTopRight.X - _nCornerTolerance) && p.X <= (pTopRight.X + _nCornerTolerance));

                            float fBottomRightConfidence = 0f;
                            if (pBottomRight != null)
                            {
                                float fInlineConfidenceX = 100f - ((float)Math.Abs(pTopRight.X - pBottomRight.X) * 5f);
                                float fInlineConfidenceY = 100f - ((float)Math.Abs(pBottomLeft.Y - pBottomRight.Y) * 5f);

                                fBottomRightConfidence = (fInlineConfidenceX + fInlineConfidenceY) / 2f;
                            }
                            else
                                continue;

                            //Increased so that the zone is not covering the box but surrounding it
                            Rectangle rect = new Rectangle(pTopLeft.X - 1, pTopLeft.Y - 1, (pTopRight.X - pTopLeft.X) + 2, (pBottomLeft.Y - pTopLeft.Y) + 2);

                            float fRectConfidence = (fTopRightConfidence + fBottomLeftConfidence + fBottomRightConfidence) / 3;

                            float fConfidence = (((pTopLeft.Confidence + pTopRight.Confidence + pBottomLeft.Confidence + pBottomRight.Confidence) * 2f) + fRectConfidence) / 9f;

                            if (!zlZones.Any(z => z.Location.X <= (rect.X + _nCornerTolerance) && z.Location.X >= (rect.X - _nCornerTolerance) && z.Location.Y <= (rect.Y + _nCornerTolerance) && z.Location.Y >= (rect.Y - _nCornerTolerance)))
                                zlZones.Add(new OMRZone(rect, fConfidence));
                        }
                    }
                }
                catch
                {
                    //if exception, process other corners
                    continue;
                }
            }

            return zlZones;
        }
    }
}
Improve this question
\$\endgroup\$

1 Answer 1

Reset to default
10
\$\begingroup\$

Naming

Your constants don't follow C# naming conventions. The convention is to use PascalCase.

Additionally you appear to be using Hungarian notation on your private fields, which also is against convention.

Design

Your dispose function does not appear to do any actual disposing that would not be performed by the garbage collector.

You have Setter methods. This is a Java paradigm that is accounted for with Properties in C#.

These two if statements should have their conditions combined:

if (verticalLine.StartPoint.X <= (horizontalLine.StartPoint.X + _nCornerTolerance) && verticalLine.StartPoint.X >= (horizontalLine.StartPoint.X - _nCornerTolerance))
                {
                    if (horizontalLine.StartPoint.Y <= (verticalLine.StartPoint.Y + _nCornerTolerance) && horizontalLine.StartPoint.Y >= (verticalLine.StartPoint.Y - _nCornerTolerance))

Code to interfaces

Your method:

private Pixel[] FindTopLeftCorners(Line[] horizontalLines, Line[] verticalLines)

Could easily take two IEnumerable<Line> interfaces instead of Line[]. This allows the caller to supply any data structure that supports enumeration (which is all you use those parameters for).

Var

You should use implicit typing when the right-hand side of the assignment makes the type obvious. For example:

Rectangle rect = new Rectangle(pTopLeft.X - 1, pTopLeft.Y - 1, (pTopRight.X - pTopLeft.X) + 2, (pBottomLeft.Y - pTopLeft.Y) + 2);

Should be:

var rect = new Rectangle(pTopLeft.X - 1, pTopLeft.Y - 1, (pTopRight.X - pTopLeft.X) + 2, (pBottomLeft.Y - pTopLeft.Y) + 2);

Generic Exceptions

Don't catch them. You can't handle a generic exception because you don't know what it is. Catch what you know you can handle, leave the rest.

This is a pretty egregious use of exceptions as flow control:

try
        {
            if (!_bImageOpen)
                throw new Exception("An image must be opened first using the 'OpenImage' method!");

            return ExtractZones();
        }
        catch (Exception ex)
        {
            return null;
        }

You throw and immediately catch an exception here, causing a return of null. Instead, return null if the image is not open.

if (!_bImageOpen)
    return null;
Improve this answer
\$\endgroup\$
2
  • 1
    \$\begingroup\$ I wouldn't go so far as to say "should use implicit typing", you can use it, but whether or not you should is not really arguable either way. \$\endgroup\$
    – Der Kommissar
    Nov 25, 2015 at 15:21
  • 1
    \$\begingroup\$ That is true and a lot of this will depend on the coding guidelines you're following. For reference, my answer follows Microsoft's style guidelines which recommends implicit typing, but they are not necessarily the perfect choice for everybody. However if you do wish to follow Microsoft's Guidelines I would say you should follow all of them for consistency. \$\endgroup\$
    – Nick Udell
    Nov 25, 2015 at 15:37

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.