From: Matthew Kerwin <matthew@...>
Date: 2013-02-03T15:49:44+09:00
Subject: Re: Class for data analysis: File.open once

--047d7b2e40b89a689204d4cc616d
Content-Type: text/plain; charset=ISO-8859-1

On 3 February 2013 11:13, Soichi Ishida <lists@ruby-forum.com> wrote:

> ruby 1.9.3p362 (2012-12-25 revision 38607) [x86_64-darwin12.2.1]
>
> I am trying to analyze a file and extract necessary data from it.
> First I need to count the total number of lines and the blank lines. the
> class and methods follow.
>
> class DataExtractor
>     attr_reader :file_name
>     def initialize(file)
>         @file_name = file
>     end
>
>     def total_lines
>         @lines = 0
>         f = File.open(@file_name, 'r')
>         f.read.each_line do |l|
>             @lines += 1
>         end
>         f.close
>         @lines
>     end
>
>     def total_blank_lines
>         regEx = /^[\s]*$\n/
>         @total_blank_lines = 0
>         f = File.open(@file_name, 'r')
>         f.read.each_line do | line |
>             if line =~ regEx
>                 @total_blank_lines += 1
>             end
>         end
>         f.close
>         @total_blank_lines
>     end
>     ....
>
> The problem is that each method has 'File.open...each_line do ' so the
> whole loop gets executed separately.
>
> Don't you think this is inefficient?  Is there better ways to develop
> methods so that the loop is needed only once?
>

Yep, sure do; and yep, sure is.  The fact that you're using @variables
inside your methods is already a hint at how to improve things -- have the
object remember them.  My first approach would be to do all the file
analysis in the constructor, sort of like this (using your code; it would
look a little different if I wrote it for myself):

    class DataExtractor
      attr_reader :file_name, :lines, :total_blank_lines
      def initialize(file)
        @file_name = file
        @lines = 0
        @total_blank_lines = 0
        regEx = /^[\s]*$\n/
        f = File.open(@file_name, 'r')
        f.read.each_line do | line |
          if line =~ regEx
            @total_blank_lines += 1
          end
          @lines += 1
        end
        f.close
      end
    end

My next approach would be to lazy-initialise the data, because that's just
a thing I like to do.  It means the lines aren't counted until they're
needed, in case that's an improvement. (Again, rough code, untested.)

    class DataExtractor
      attr_reader :file_name

      def initialize(file)
        @file_name = file
        @lines = -1
        @total_blank_lines = -1
      end

      def lazy_init_lines
        regEx = /^[\s]*$\n/
        f = File.open(@file_name, 'r')
        f.read.each_line do | line |
          if line =~ regEx
            @total_blank_lines += 1
          end
          @lines += 1
        end
        f.close
      end

      def lines
        lazy_init_lines if @lines < 0
        @lines
      end

      def total_blank_lines
        lazy_init_lines if @total_blank_lines < 0
        @total_blank_lines
      end
    end

In either case you perform a single open-read-close loop, and can access
the totals over and over again.  Various further optimisations and
improvements are available to be made at your discretion.

-- 
  Matthew Kerwin, B.Sc (CompSci) (Hons)
  http://matthew.kerwin.net.au/
  ABN: 59-013-727-651

  "You'll never find a programming language that frees
  you from the burden of clarifying your ideas." - xkcd

--047d7b2e40b89a689204d4cc616d
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr">On 3 February 2013 11:13, Soichi Ishida <span dir=3D"ltr">=
&lt;<a href=3D"mailto:lists@ruby-forum.com" target=3D"_blank">lists@ruby-fo=
rum.com</a>&gt;</span> wrote:<br><div class=3D"gmail_extra"><div class=3D"g=
mail_quote">

<blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-=
left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;p=
adding-left:1ex">ruby 1.9.3p362 (2012-12-25 revision 38607) [x86_64-darwin1=
2.2.1]<br>

<br>
I am trying to analyze a file and extract necessary data from it.<br>
First I need to count the total number of lines and the blank lines. the<br=
>
class and methods follow.<br>
<br>
class DataExtractor<br>
=A0 =A0 attr_reader :file_name<br>
=A0 =A0 def initialize(file)<br>
=A0 =A0 =A0 =A0 @file_name =3D file<br>
=A0 =A0 end<br>
<br>
=A0 =A0 def total_lines<br>
=A0 =A0 =A0 =A0 @lines =3D 0<br>
=A0 =A0 =A0 =A0 f =3D File.open(@file_name, &#39;r&#39;)<br>
=A0 =A0 =A0 =A0 f.read.each_line do |l|<br>
=A0 =A0 =A0 =A0 =A0 =A0 @lines +=3D 1<br>
=A0 =A0 =A0 =A0 end<br>
=A0 =A0 =A0 =A0 f.close<br>
=A0 =A0 =A0 =A0 @lines<br>
=A0 =A0 end<br>
<br>
=A0 =A0 def total_blank_lines<br>
=A0 =A0 =A0 =A0 regEx =3D /^[\s]*$\n/<br>
=A0 =A0 =A0 =A0 @total_blank_lines =3D 0<br>
=A0 =A0 =A0 =A0 f =3D File.open(@file_name, &#39;r&#39;)<br>
=A0 =A0 =A0 =A0 f.read.each_line do | line |<br>
=A0 =A0 =A0 =A0 =A0 =A0 if line =3D~ regEx<br>
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 @total_blank_lines +=3D 1<br>
=A0 =A0 =A0 =A0 =A0 =A0 end<br>
=A0 =A0 =A0 =A0 end<br>
=A0 =A0 =A0 =A0 f.close<br>
=A0 =A0 =A0 =A0 @total_blank_lines<br>
=A0 =A0 end<br>
=A0 =A0 ....<br>
<br>
The problem is that each method has &#39;File.open...each_line do &#39; so =
the<br>
whole loop gets executed separately.<br>
<br>
Don&#39;t you think this is inefficient? =A0Is there better ways to develop=
<br>
methods so that the loop is needed only once?<br></blockquote><div><br></di=
v><div>Yep, sure do; and yep, sure is. =A0The fact that you&#39;re using @v=
ariables inside your methods is already a hint at how to improve things -- =
have the object remember them. =A0My first approach would be to do all the =
file analysis in the constructor, sort of like this (using your code; it wo=
uld look a little different if I wrote it for myself):</div>
<div><br></div><div style>=A0 =A0 class DataExtractor</div><div style>=A0 =
=A0 =A0 attr_reader :file_name, :lines, :total_blank_lines</div><div style>=
=A0 =A0 =A0 def initialize(file)</div><div style>=A0 =A0 =A0 =A0 @file_name=
 =3D file</div><div style>
=A0 =A0 =A0 =A0 @lines =3D 0</div><div style>=A0 =A0 =A0 =A0 @total_blank_l=
ines =3D 0</div>=A0 =A0 =A0 =A0 regEx =3D /^[\s]*$\n/<br>=A0 =A0 =A0 =A0 f =
=3D File.open(@file_name, &#39;r&#39;)<br>=A0 =A0 =A0 =A0 f.read.each_line =
do | line |<br>=A0 =A0 =A0 =A0 =A0 if line =3D~ regEx<br>
=A0 =A0 =A0 =A0 =A0 =A0 @total_blank_lines +=3D 1<br>=A0 =A0 =A0 =A0 =A0 en=
d</div><div class=3D"gmail_quote">=A0 =A0 =A0 =A0 =A0 @lines +=3D 1<br>=A0 =
=A0 =A0 =A0 end<br>=A0 =A0 =A0 =A0 f.close<br><div style>=A0 =A0 =A0 end<br=
></div><div style>=A0 =A0 end</div>
</div><div class=3D"gmail_extra"><br></div>My next approach would be to laz=
y-initialise the data, because that&#39;s just a thing I like to do. =A0It =
means the lines aren&#39;t counted until they&#39;re needed, in case that&#=
39;s an improvement. (Again, rough code, untested.)<br clear=3D"all">
<div><br></div><div><div class=3D"gmail_quote"><div>=A0 =A0 class DataExtra=
ctor</div><div>=A0 =A0 =A0 attr_reader :file_name</div><div>=A0 =A0=A0</div=
><div>=A0 =A0 =A0 def initialize(file)</div><div>=A0 =A0 =A0 =A0 @file_name=
 =3D file</div><div>=A0 =A0 =A0 =A0 @lines =3D -1<br>
</div><div>=A0 =A0 =A0 =A0 @total_blank_lines =3D -1</div><div style>=A0 =
=A0 =A0 end</div><div>=A0 =A0=A0</div><div style>=A0 =A0 =A0 def lazy_init_=
lines</div>=A0 =A0 =A0 =A0 regEx =3D /^[\s]*$\n/<br>=A0 =A0 =A0 =A0 f =3D F=
ile.open(@file_name, &#39;r&#39;)<br>=A0 =A0 =A0 =A0 f.read.each_line do | =
line |<br>
=A0 =A0 =A0 =A0 =A0 if line =3D~ regEx<br>=A0 =A0 =A0 =A0 =A0 =A0 @total_bl=
ank_lines +=3D 1<br>=A0 =A0 =A0 =A0 =A0 end</div><div class=3D"gmail_quote"=
>=A0 =A0 =A0 =A0 =A0 @lines +=3D 1<br>=A0 =A0 =A0 =A0 end<br>=A0 =A0 =A0 =
=A0 f.close<br><div>=A0 =A0 =A0 end<br></div><div>=A0 =A0=A0</div><div>=A0 =
=A0 =A0 def lines</div>
<div>=A0 =A0 =A0 =A0 lazy_init_lines if @lines &lt; 0</div><div>=A0 =A0 =A0=
 =A0 @lines</div><div>=A0 =A0 =A0 end</div><div>=A0 =A0=A0</div><div>=A0 =
=A0 =A0 def total_blank_lines</div><div>=A0 =A0 =A0 =A0 lazy_init_lines if =
@total_blank_lines &lt; 0</div><div>=A0 =A0 =A0 =A0 @total_blank_lines</div=
>
<div>=A0 =A0 =A0 end</div><div>=A0 =A0 end<br></div><div><br></div><div sty=
le>In either case you perform a single open-read-close loop, and can access=
 the totals over and over again. =A0Various further optimisations and impro=
vements are available to be made at your discretion.</div>
<div><br></div></div></div>-- <br>=A0 Matthew Kerwin, B.Sc (CompSci) (Hons)=
<br>=A0 <a href=3D"http://matthew.kerwin.net.au/" target=3D"_blank">http://=
matthew.kerwin.net.au/</a><br>=A0 ABN: 59-013-727-651<br><br>=A0 &quot;You&=
#39;ll never find a programming language that frees<br>

=A0 you from the burden of clarifying your ideas.&quot; - xkcd
</div></div>

--047d7b2e40b89a689204d4cc616d--