Codereview #2 of 3 (Porro) from 16 (Perali)

[thomasporro]

Hi,
I've loaded a lot of files, but the methods to review are:
-detectTextRegions and extractTextFromBitmap in ExtractTheText.java
-editBright and isBright in PreProcessing.java
I've put into the pull request also the interface so you can understand more easily the methods but they are not to be revised
Thank You

[Fisher4537]

Well, I can see that you work hard and I appreciate it so much. Indeed the code is full of clear comments and the methods are unit testable.

In any case, I don't agree with the methods declaration in the interface.
You describe the interface purpose like that: "Interface used to detect the text in an image" but in the code it seems that you are only looking for the cropped images were there is some text.
if I were you, I would do an interface method that take the bitmap and the DetectTheTextMethods as parameter, and return a List object which hopefully have some text inside because outside we have an OCR module which take bitmap as input and return text as output.
In the class you can implement all the methods to modify the image or the cropped image using directly the Mat object, improving the performance (*).
Moreover I think that this method should throw an exception if it can't convert the input image, the exception could be managed in different way outside the module's code (e.g. passing the complete image to the OCR module).

(*) if you apply detectTextRegions and than extractTextFromBitmap you perform 3 conversions with conversionBitmapToMat and conversionMatToBitmap methods. Instead you could get the cropped bitmap with 2 conversions if you take in as parameter a Mat image. If you want to encapsulate this implementation detail you can modify detectTextRegions method to return a List object and delete extractTextFromBitmap from the interface. In code word, the new interface would be like this:

public interface DetectTheText {
    /**
     * Detects all the regions where there's some text in the image
     * @param image The image we want to analyze. Not null.
     * @param method The method used to extract the text area. See DetectTheTextMethods.java.
     * @return The TextAreas object that contains the area where there's some text. 
     *         If it fails trying to convert images, throw ConversionFailedException.
     *         if it fails to find text return List<Bitmap> containing the only the full image.
     */
    List<Bitmap> getCroppedBitmapWithText(Bitmap image, DetectTheTextMethods method) throws ConversionFailedException;

[thomasporro]

Maybe the specs of the interface aren't clear enough. The original reason why the interfaces have 2 methods it's because we thought to implement a new method to extract the text using another library, but we don't succeded . Actually your advice to modify the interface so it can have only one method is very good, I'll looking to implement it in the next commit.
We had the thought of passing only one time the bitmap to avoid a lot of conversions, but unfortunately we had already committed the review.
I appriciate that you actually read all the interfaces to understand more the code.
Thank for your review, I will update the code as soon as I can.