Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

As part 2 of my other question concerning this long running project I've been inquiring about on CR, I reimplemented my generic function for determining if an array of arrays is a sub-array or submatrix of a larger array of arrays.

I'm looking for advice on better implementation, ways to improve code clarity, advice on best practices and anything else that might be useful. This is my new implementation:


namespace MathAlgorithms

module ArrayFunctions = 
    // Generic because it makes testing easier, yay math
    let IsSubMatrix (smallArray:'a[][]) (largeArray:'a[][]) (startCoordinate:(int * int)) =
        let searchHeight , searchWidth = smallArray.Length - 1 , smallArray.[0].Length - 1
        let startHeight  , startWidth  = startCoordinate

        try 
            let WidthLoop heightIndex = 
                let rec WidthLoopRec heightIndex widthIndex =
                    let largeValue = largeArray.[startHeight + heightIndex].[startWidth + widthIndex]
                    let smallValue = smallArray.[heightIndex].[widthIndex]

                    match ( smallValue = largeValue , widthIndex < searchWidth ) with
                    | ( true  , true  ) -> WidthLoopRec heightIndex (widthIndex + 1)
                    | ( true  , false ) -> true 
                    | ( false , _     ) -> false
                WidthLoopRec heightIndex 0

            let HeightLoop () =
                let rec HeightLoopRec heightIndex = 
                    let isMatch = WidthLoop heightIndex

                    match ( isMatch , heightIndex < searchHeight) with
                    | ( true  , true  ) -> HeightLoopRec ( heightIndex + 1 )
                    | ( true  , false ) -> true
                    | ( false , _     ) -> false
                HeightLoopRec 0

            HeightLoop ()

        with // Not really sure what I want to do with error handling atm
            | :? System.ArgumentOutOfRangeException -> false
            | :? System.ArgumentNullException       -> false
            | :? System.ArgumentException           -> false

I ran this code through my tests, and it's functionally the same as the old while-do version which I'm including for comparison's sake:


namespace MathAlgorithms

module ArrayFunctions = 
    // Generic because it makes testing easier, yay math
    let SearchSubset (tSmallArray:'a[][]) (tLargeArray:'a[][]) (pCoordinate:(int * int)) =
        let tSmallHeight = tSmallArray.Length
        let tSmallWidth = tSmallArray.[0].Length

        let tHeightIndex = fst pCoordinate
        let tWidthIndex = snd pCoordinate
        let mutable tSmallHeightIndex = 0
        let mutable tSmallWidthIndex = 0
        let mutable tMatch = true

        try 
            while ( tSmallHeightIndex < tSmallHeight - 1 ) && tMatch do
                while ( tSmallWidthIndex < tSmallWidth - 1 ) && tMatch do
                    let tLargeCurrentValue = tLargeArray.[tHeightIndex + tSmallHeightIndex].[tWidthIndex + tSmallWidthIndex]
                    let tSmallCurrentValue = tSmallArray.[tSmallHeightIndex].[tSmallWidthIndex]

                    if tSmallCurrentValue = tLargeCurrentValue then
                        tSmallWidthIndex <- tSmallWidthIndex + 1         
                    else
                        tMatch <- false

                tSmallWidthIndex  <- 0
                tSmallHeightIndex <- tSmallHeightIndex + 1

            tMatch
        with
            | _ -> false

Github for further info:

https://github.com/Kenneth-Posey/LearningFsharp/tree/master/FsharpTutorial/YeFsharpLibrary

The use of the function is in the module FsharpImaging.fs, it's tested by UnitTest.fs and it's located in ArrayFunctions.fs.

share|improve this question
    
Was there a problem with the solution I posted here? Also yay, no more prefixes :) –  mjolka Aug 6 '14 at 23:19
    
Not as far as I know, and thanks for that. I just needed to learn tail recursion and this was a good way to do it. I'm going to implement your slice method as an alternative later on. –  Kenneth Posey Aug 6 '14 at 23:20
1  
Joining the declarations of start height/width makes sense because of tuple decomposition, but I'm not sure I would join the search height/width declarations. –  Dan Lyons Aug 6 '14 at 23:27
    
Yeah, I keep swinging back and forth with my opinion on that, but for now I think I'm leaving it just because it lines up nicely with the tuple unpacking. –  Kenneth Posey Aug 6 '14 at 23:33

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Browse other questions tagged or ask your own question.