1
\$\begingroup\$

I'm fairly new to F# and have written a program to parse PLY files - this is however done in an imperative way with mutable values and as far as I know that should be avoided in functional languages.

The program works, but is the performance weakened by this imperative way of doing things? How is the coding style?

module PLYparsing
open System.IO;;
open System.Collections.Generic;;
open System.Text.RegularExpressions;;

// The types in a PLY file (at least in the ones we use)
type Vertice = V of float * float * float;;
type Face = F of int * int * int;;

/// <summary>Read all lines in a file into a sequence of strings.</summary>
/// <param name="fileName">Name of file to be parsed - must be situated in Resources folder.</param>
/// <returns>A sequence of the lines in the file.</returns>
let readLines (fileName:string) = 
   let baseName = Directory.GetParent(__SOURCE_DIRECTORY__).FullName
   let fullPath = Path.Combine(baseName, ("Resources\\" + fileName))
   seq { use sr = new StreamReader (fullPath)
         while not sr.EndOfStream do
            yield sr.ReadLine() };;

// Mutable values to be assigned during parsing.
let mutable vertexCount = 0;;
let mutable faceCount = 0;;
let mutable faceProperties = ("", "");;
let mutable (vertexProperties: (string * string) list) = [];;
let mutable (objectInfo: (string * string) list) = [];;
let mutable (vertices: seq<Vertice>) = Seq.empty;;
let mutable (faces: seq<Face>) = Seq.empty;;

// Malformed lines in the PLY file? Raise this exception.
exception ParseError of string;;

/// <summary>Checks whether a string matches a certain regex.</summary>
/// <param name="s">The string to check.</param>
/// <param name="r">The regex to match.</param>
/// <returns>Whether or not the string matches the regex.</returns>
let matchesRegex s r =
   Regex.Match(s, r).Success

/// <summary>Parse the header of a PLY file into predefined, mutable values.</summary>
/// <param name="header">A sequence of the header lines in a PLY file, not including "end_header".</param>
/// <exception cref="ParseError">Raised when the input is not recognized as anything usefull.</exception>
let parseHeader (header: seq<string>) = 
   for line in header do
      let splitted = line.Split[|' '|]
      match line with
      | x when matchesRegex x @"obj_info .*"                ->
         let a = Array.item 1 splitted
         let b = Array.item 2 splitted
         objectInfo <- objectInfo@[(a, b)]
      | x when matchesRegex x @"element vertex \d*"         ->
         vertexCount <- int (Array.item 2 splitted)
      | x when matchesRegex x @"property list .*"           ->
         let a = Array.item 2 splitted
         let b = Array.item 3 splitted
         faceProperties <- (a, b)
      | x when matchesRegex x @"property .*"                ->
         let a = Array.item 1 splitted
         let b = Array.item 2 splitted
         vertexProperties <- vertexProperties@[(a, b)]
      | x when matchesRegex x @"element face \d*"           ->
         faceCount <- int (Array.item 2 splitted)
      | x when ((x = "ply") || matchesRegex x @"format .*") -> ()
      | _                                                   -> 
         System.Console.WriteLine(line)
         raise (ParseError("Malformed header."));;

/// <summary>Convert a string to a vertice.</summary>
/// <param name="s">String containing a vertice.</param>
/// <returns>The converted vertice.</returns>
/// <exception cref="ParseError">Raised when the length of the input string is less that 3.</exception>
let stringToVertice (s: string) =
   match s with
   | s when s.Length < 3 -> System.Console.WriteLine(s)
                            raise (ParseError("Malformed vertice."))
   | _                   -> let splitted = s.Split[|' '|]
                            let x = Array.item 0 splitted
                            let y = Array.item 1 splitted
                            let z = Array.item 2 splitted
                            V(float x, float y, float z);;

/// <summary>Convert a sequence of strings to a sequence of vertices.</summary>
/// <param name="vertices">Sequence of strings to convert.</param>
/// <returns>A sequence of the converted sequences.</returns>
let parseVertices (vertices: seq<string>) =
   Seq.map(fun a -> stringToVertice(a)) vertices;;

/// <summary>Convert a string to a face.</summary>
/// <param name="s">String containing a face.</param>
/// <returns>The converted face.</returns>
/// <exception cref="ParseError">Raised when the length of the input string is less that 3.</exception>
let stringToFace (s: string) =
   match s with
   | s when s.Length < 3 -> System.Console.WriteLine(s)
                            raise (ParseError("Malformed face."))
   | _                   -> let splitted = s.Split[|' '|]
                            let x = Array.item 0 splitted
                            let y = Array.item 1 splitted
                            let z = Array.item 2 splitted
                            F(int x, int y, int z);;

/// <summary>Convert a sequence of strings to a sequence of faces.</summary>
/// <param name="faces">Sequence of strings to convert.</param>
/// <returns>A sequence of the converted faces.</returns>
let parseFaces (faces: seq<string>) =
      Seq.map(fun a -> stringToFace(a)) faces;;

/// <summary>Main function in PLY parsing. Calls all helper functions and assigns the required mutable values.</summary>
/// <param name="fileName">File to be parsed - name of file in Resources folder.</param>
let parsePLYFile fileName =
   let lines = readLines fileName
   // At which index is the header located? The vertices? The faces?
   let bodyPos = lines |> Seq.findIndex(fun a -> a = "end_header")
   let header = lines |> Seq.take bodyPos
   parseHeader header
   let vertexPart = lines |> Seq.skip (bodyPos + 1) |> Seq.take vertexCount
   let facePart = (lines |> Seq.skip (bodyPos + vertexCount + 1) |> Seq.take faceCount)
   // Parse the header, the vertices & the faces.
   vertices <- parseVertices vertexPart
   faces <- parseFaces facePart;;
\$\endgroup\$
  • \$\begingroup\$ "how is the performance?" Did you benchmark it? \$\endgroup\$ – Mast Apr 8 '16 at 13:25
  • \$\begingroup\$ Nope, haven't done it - it wasn't a question of the actual performance, more like a question of how it was compared to doing it in some other, maybe more 'functional', way. \$\endgroup\$ – zniwalla Apr 8 '16 at 13:51
  • \$\begingroup\$ Don't have the time to do a full review, but it's "functional" enough. You can remove all the conditional guards and replace all the Array.item usages with parameterized active patterns. Just make sure the matchesRegex function stays generic enough and one-off logic doesn't start leaking in. \$\endgroup\$ – moarboilerplate Apr 11 '16 at 7:14
  • \$\begingroup\$ @moarboilerplate how would you replace Array.item with parameterized active patterns? Also, I think it would be possible to replace the mutable variables by using recursion and folding instead to get a fully functional solution without any side effects? \$\endgroup\$ – user102568 Apr 11 '16 at 10:00
  • \$\begingroup\$ @TVANO Using the example in the link, split first, and then pattern match against the split array using the regex and parameters. You should be able to use wildcards if you can't guarantee everything in the split array. The way I would work towards a better solution would be to do away with the mutable variables by simply changing the readLines method to return a tuple of 2 collections, one collection containing the faces, and one containing the vertices. That should get the code towards a more workable state (and maybe even reduce the need for readLines to be a separate method). \$\endgroup\$ – moarboilerplate Apr 11 '16 at 10:23
0
\$\begingroup\$

That is how I would write it without exceptions and mutable states. I'm still learning so it might be done shorter, or more efficient. Have a look at https://fsharpforfunandprofit.com/rop/.

Edit: Exceptions are treated as bad style in functional programmimg because they are not represented by the signature of the function. For Example: (Apple -> Banana -> Cherry) is a function that takes an Apple and a Bannan and gives back a Cherry. If this function rise an exception this is not obvious. In a pure language like Haskel I think it is not possible.

Mutable states can have side effects if used in parallel programming and with parallel programming you might improve the perfomance of parseVertices and parseFaces by adding async{...}. To avoid mutable state I imaginetto wrap my data in brown paper and throw away or forget the old data. I hope the Compiler can handle the performance, at least if I use tail recursion. The easy use of async is the reward for avoiding mutable states.

In the project I work correct code and developing time are more importend than performance, because they are for a small number of users. For me F# fits there perfect.

Edit: The code should no work without stackoverflow.

module PLYparsing
open System.IO;;
open System.Text.RegularExpressions;;

// The types in a PLY file (at least in the ones we use)
type Vertice = V of float * float * float;;
type Face = F of int * int * int;;

/// <summary>Read all lines in a file into a sequence of strings.</summary>
/// <param name="fileName">Name of file to be parsed - must be situated in Resources folder.</param>
/// <returns>A sequence of the lines in the file.</returns>
let readLines (fileName:string) = 
   let baseName = Directory.GetParent(__SOURCE_DIRECTORY__).FullName
   let fullPath = Path.Combine(baseName, ("Resources\\" + fileName))
   seq { use sr = new StreamReader (fullPath)
         while not sr.EndOfStream do
            yield sr.ReadLine() };;

// not-mutable values to be assigned during parsing.
type ParserResult = 
    {
        VertexCount : int
        FaceCount : int
        FaceProperties : string * string
        VertexProperties : (string * string) list
        ObjectInfo: (string * string) list
        Vertices: seq<Vertice>
        Faces: seq<Face>
    }
    static member Init()=   
        {
            VertexCount = 0
            FaceCount = 0
            FaceProperties = ("","")
            VertexProperties =[]
            ObjectInfo = []
            Vertices = Seq.empty
            Faces = Seq.empty
        }
// Malformed lines in the PLY file? Raise this exception.
type ParserSuccess<'a> =
| Success of 'a
| Failure of string

let map f aPS=
    match aPS with 
    | Success( a )-> f a |> Success
    | Failure s -> Failure s

let combine xPS yPS = 
    match (xPS,yPS) with
    | Success(x),Success(y) -> Success(x,y)
    | _ -> Failure <| sprintf "Can not combine %A %A" xPS yPS

let bind f aPS = 
    match aPS with
    | Success x -> f x
    | Failure s -> Failure s

let outerSuccess<'a> (seqIn: ParserSuccess<'a> seq) = 
    let containsFailure = 
        seqIn
        |>Seq.exists (fun (elPS) -> 
                match elPS with 
                | Failure _ -> true
                | _ -> false)
    match containsFailure with
    | true ->
        Failure ("Could be a litte bit more precise: Failure in " + (typeof<'a>).ToString())
    | false ->
        Success( Seq.map (fun s -> match s with | Success(v) -> v ) seqIn)




//exception ParseError of string;;

/// <summary>Checks whether a string matches a certain regex.</summary>
/// <param name="s">The string to check.</param>
/// <param name="r">The regex to match.</param>
/// <returns>Whether or not the string matches the regex.</returns>
let matchesRegex s r =
   Regex.Match(s, r).Success

/// <summary>Parse the header of a PLY file into predefined, mutable values.</summary>
/// <param name="header">A sequence of the header lines in a PLY file, not including "end_header".</param>
/// <exception cref="ParseError">Raised when the input is not recognized as anything useful.</exception>
let parseHeader (header: seq<string>) = 

    let parseHeaderRaw accPS (line:string) = 
      match accPS with 
      | Failure (_) -> accPS
      | Success (parserResult) -> 
          let splitted = line.Split[|' '|]
          match line with
          | x when matchesRegex x @"obj_info .*"                ->
             let a = Array.item 1 splitted
             let b = Array.item 2 splitted
             {  parserResult with ObjectInfo = parserResult.ObjectInfo@[(a, b)]} |> Success
          | x when matchesRegex x @"element vertex \d*"         ->
             {  parserResult with VertexCount =  int (Array.item 2 splitted)} |> Success
          | x when matchesRegex x @"property list .*"           ->
             let a = Array.item 2 splitted
             let b = Array.item 3 splitted
             {  parserResult with FaceProperties =  (a, b)}
             |> Success
          | x when matchesRegex x @"property .*"                ->
             let a = Array.item 1 splitted
             let b = Array.item 2 splitted
             {  parserResult with VertexProperties =  parserResult.VertexProperties@[(a, b)]}
             |> Success
          | x when matchesRegex x @"element face \d*"           ->
             {  parserResult with FaceCount =  int (Array.item 2 splitted)}
             |> Success
          | x when ((x = "ply") || matchesRegex x @"format .*") -> Success parserResult
          | _                                                   -> 
             Failure "Malformed header."  

    header
    |> Seq.fold parseHeaderRaw  (ParserResult.Init() |> Success)

/// <summary>Convert a string to a vertice.</summary>
/// <param name="s">String containing a vertice.</param>
/// <returns>The converted vertice.</returns>
/// <exception cref="ParseError">Raised when the length of the input string is less that 3.</exception>
let stringToVertice (s: string) =
   match s with
   | s when s.Length < 3 -> System.Console.WriteLine(s)
                            sprintf "Malformed vertices: %s" s |> Failure 
   | _                   -> let splitted = s.Split[|' '|]
                            let pick  i = Array.item i splitted
                            let x = pick 0
                            let y = pick 1
                            let z = pick 2
                            V(float x, float y, float z) |> Success

/// <summary>Convert a sequence of strings to a sequence of vertices.</summary>
/// <param name="vertices">Sequence of strings to convert.</param>
/// <returns>A sequence of the converted sequences.</returns>
let parseVertices (vertices: seq<string>) =
   Seq.map stringToVertice vertices 
   |> outerSuccess


/// <summary>Convert a string to a face.</summary>
/// <param name="s">String containing a face.</param>
/// <returns>The converted face.</returns>
/// <exception cref="ParseError">Raised when the length of the input string is less that 3.</exception>
let stringToFace (s: string) =
   match s with
   | s when s.Length < 3 -> System.Console.WriteLine(s)
                            sprintf "Malformed vertices: %s" s |> Failure                             
   | _                   -> let splitted = s.Split[|' '|]
                            let x = Array.item 0 splitted
                            let y = Array.item 1 splitted
                            let z = Array.item 2 splitted
                            F(int x, int y, int z) |> Success

/// <summary>Convert a sequence of strings to a sequence of faces.</summary>
/// <param name="faces">Sequence of strings to convert.</param>
/// <returns>A sequence of the converted faces.</returns>
let parseFaces (faces: seq<string>) =
      faces 
      |> Seq.map stringToFace 
      |> outerSuccess

/// <summary>Main function in PLY parsing. Calls all helper functions and assigns the required mutable values.</summary>
/// <param name="fileName">File to be parsed - name of file in Resources folder.</param>
let parsePLYFile fileName =
   let lines = readLines fileName
   // At which index is the header located? The vertices? The faces?
   let bodyPos = lines |> Seq.findIndex(fun a -> a = "end_header")
   let header = lines |> Seq.take bodyPos

   // Parse the header, the vertices & the faces.
   parseHeader header
   |> bind (fun resultHeaderPS -> 
        let faces = lines |> Seq.skip (bodyPos + resultHeaderPS.VertexCount + 1) |> Seq.take  resultHeaderPS.FaceCount |> parseFaces
        let vertices = lines |> Seq.skip (bodyPos + 1) |> Seq.take resultHeaderPS.VertexCount |> parseVertices

        combine vertices faces
        |> map(fun (vertices, faces) ->
                {   resultHeaderPS with Vertices = vertices; Faces = faces }  ) )
\$\endgroup\$
  • \$\begingroup\$ While you describe what you've done with the OP code, it sure would benefit if you had a bit of meat to your answer. Maybe explain why mutable states and exceptions should be avoided etc. \$\endgroup\$ – Marc-Andre Apr 15 '16 at 12:57
  • \$\begingroup\$ It was something exactly like this I was looking for! Thank you so much for taking some time to make this - I'm grateful. \$\endgroup\$ – zniwalla Apr 19 '16 at 7:21
  • \$\begingroup\$ I've implemented your work, parsing a big .PLY file now gives me StackOverflowException. Just wanted to tell you, no worries. \$\endgroup\$ – zniwalla Apr 21 '16 at 8:25
  • \$\begingroup\$ The stackoverflow happens in the last line of code, so I have no quick answer. \$\endgroup\$ – Peter Siebke Apr 21 '16 at 13:18
  • \$\begingroup\$ It is my outerSuccess function that produce th stackoverflow. I will write a better one. \$\endgroup\$ – Peter Siebke Apr 21 '16 at 20:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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