Badly organized code?

Discussion in 'Ruby' started by Martin Hansen, May 27, 2010.

  1. Martin Hansen, May 27, 2010
    #1
    1. Advertising

  2. 2010/5/27 Martin Hansen <>:
    > I am writing generic command line scripts and here is a minimal example:
    > http://pastie.org/979581 - which I am quite happy with. Now the required
    > 'biopieces' class is here http://pastie.org/979577 - but this I am not
    > really happy with. Mainly, I think it is poorly organized? Suggestions?


    I would extract the opion parsing from that class and either use a
    Hash, OpenStruct or custom class for transportation of your options.
    That way you make your bio pieces processing independent from the
    interface (command line, graphical application etc.).

    Then I would extend the parsing algorithm and offer a similar
    interface like CSV or File along the lines of:

    class BioPieces
    include Enumerable

    def self.open(file_name)
    File.open file_name do |io|
    yield new io
    end
    end

    def self.foreach(file_name, &b)
    open file_name do |bp|
    bp.each_record(&b)
    end
    end

    def initialize(io)
    @io = io
    end

    def each_record
    rec = {}

    @io.each_line do |line|
    case line
    when /^([^:])+:\s*(.*)$/
    rec[$1.strip] = $2.strip
    when /^-+$/
    yield rec
    rec = {}
    else
    raise "Invalid input: %p" % line
    end
    end

    # maybe add this:
    # yield rec unless rec.empty?

    self
    end

    alias each each_record
    end

    Btw, your code looks pretty clean and well documented. That's good!

    Kind regards

    robert

    --
    remember.guy do |as, often| as.you_can - without end
    http://blog.rubybestpractices.com/
    Robert Klemme, May 27, 2010
    #2
    1. Advertising

  3. Hi Robert,


    It is quite important to me that other people can put together a
    "Biopiece" as easily as possible i.e. expand the minimal example.
    Therefore I have moved code which under normal circumstances would be
    placed in the 'Biopieces' script to the Biopieces class - and I quite
    possibly violate a number of conventions that way.

    > I would extract the opion parsing from that class and either use a
    > Hash, OpenStruct or custom class for transportation of your options.
    > That way you make your bio pieces processing independent from the
    > interface (command line, graphical application etc.).


    I really don't want upcoming 'Biopiece writers' to fiddle around with
    other classes than the Biopieces class for the sake of simplicity - and
    I am willing to have option parsing and record parsing mixed even though
    it is ugly.

    > Btw, your code looks pretty clean and well documented. That's good!


    Thanks, I am working hard on it!


    I was wondering if nested classes might bring some order to this?


    Cheers,


    Martin
    --
    Posted via http://www.ruby-forum.com/.
    Martin Hansen, May 27, 2010
    #3
  4. Martin Hansen

    Josh Cheek Guest

    [Note: parts of this message were removed to make it a legal post.]

    On Thu, May 27, 2010 at 7:26 AM, Martin Hansen <> wrote:

    > Hi Robert,
    >
    >
    > It is quite important to me that other people can put together a
    > "Biopiece" as easily as possible i.e. expand the minimal example.
    > Therefore I have moved code which under normal circumstances would be
    > placed in the 'Biopieces' script to the Biopieces class - and I quite
    > possibly violate a number of conventions that way.
    >
    > > I would extract the opion parsing from that class and either use a
    > > Hash, OpenStruct or custom class for transportation of your options.
    > > That way you make your bio pieces processing independent from the
    > > interface (command line, graphical application etc.).

    >
    > I really don't want upcoming 'Biopiece writers' to fiddle around with
    > other classes than the Biopieces class for the sake of simplicity - and
    > I am willing to have option parsing and record parsing mixed even though
    > it is ugly.
    >
    > > Btw, your code looks pretty clean and well documented. That's good!

    >
    > Thanks, I am working hard on it!
    >
    >
    > I was wondering if nested classes might bring some order to this?
    >
    >
    > Cheers,
    >
    >
    > Martin
    > --
    > Posted via http://www.ruby-forum.com/.
    >
    >

    If you have a comprehensive set of tests, then people can refactor without
    fear of breaking.
    Josh Cheek, May 27, 2010
    #4
  5. Josh Cheek wrote:
    > On Thu, May 27, 2010 at 7:26 AM, Martin Hansen <> wrote:
    >
    >> > Hash, OpenStruct or custom class for transportation of your options.

    >> Thanks, I am working hard on it!
    >> Posted via http://www.ruby-forum.com/.
    >>
    >>

    > If you have a comprehensive set of tests, then people can refactor
    > without
    > fear of breaking.


    I am working on comprehensive unit testing of class biopieces, but
    "Biopiece writers" should not change anything in that class. They should
    only concentrate on what is in the script they are writing and whatever
    classes they write to expand that.


    Martin
    --
    Posted via http://www.ruby-forum.com/.
    Martin Hansen, May 27, 2010
    #5
  6. 2010/5/27 Martin Hansen <>:

    > It is quite important to me that other people can put together a
    > "Biopiece" as easily as possible i.e. expand the minimal example.
    > Therefore I have moved code which under normal circumstances would be
    > placed in the 'Biopieces' script to the Biopieces class - and I quite
    > possibly violate a number of conventions that way.
    >
    >> I would extract the opion parsing from that class and either use a
    >> Hash, OpenStruct or custom class for transportation of your options.
    >> That way you make your bio pieces processing independent from the
    >> interface (command line, graphical application etc.).

    >
    > I really don't want upcoming 'Biopiece writers' to fiddle around with
    > other classes than the Biopieces class for the sake of simplicity - and
    > I am willing to have option parsing and record parsing mixed even though
    > it is ugly.


    Well, then I don't know what you're after. You say you suspect it's
    badly structured, someone points out something that indicates bad
    structure and you say you want to keep it that way. What is your goal
    and what do you expect from us?

    Cheers

    robert

    --
    remember.guy do |as, often| as.you_can - without end
    http://blog.rubybestpractices.com/
    Robert Klemme, May 27, 2010
    #6
  7. The goal is to create a bunch of scripts that can be piped together
    easily on the command line in a UNIX environment to manipulate text
    based records. They scripts are supposed to be easy to use, but also
    easy to write - in whatever programming language. Have a look here:
    www.biopieces.org.

    I expect/hope to learn Ruby with your fine assistance, and I want to
    keep in mind good practices etc. However, for this particular project it
    is difficult.

    Cheers,


    Martin



    Robert Klemme wrote:
    > 2010/5/27 Martin Hansen <>:
    >
    >>
    >> I really don't want upcoming 'Biopiece writers' to fiddle around with
    >> other classes than the Biopieces class for the sake of simplicity - and
    >> I am willing to have option parsing and record parsing mixed even though
    >> it is ugly.

    >
    > Well, then I don't know what you're after. You say you suspect it's
    > badly structured, someone points out something that indicates bad
    > structure and you say you want to keep it that way. What is your goal
    > and what do you expect from us?
    >
    > Cheers
    >
    > robert


    --
    Posted via http://www.ruby-forum.com/.
    Martin Hansen, May 27, 2010
    #7
  8. Martin Hansen

    Josh Cheek Guest

    [Note: parts of this message were removed to make it a legal post.]

    On Thu, May 27, 2010 at 8:05 AM, Martin Hansen <> wrote:

    > Josh Cheek wrote:
    > > On Thu, May 27, 2010 at 7:26 AM, Martin Hansen <> wrote:
    > >
    > >> > Hash, OpenStruct or custom class for transportation of your options.
    > >> Thanks, I am working hard on it!
    > >> Posted via http://www.ruby-forum.com/.
    > >>
    > >>

    > > If you have a comprehensive set of tests, then people can refactor
    > > without
    > > fear of breaking.

    >
    > I am working on comprehensive unit testing of class biopieces, but
    > "Biopiece writers" should not change anything in that class. They should
    > only concentrate on what is in the script they are writing and whatever
    > classes they write to expand that.
    >
    >
    > Martin
    > --
    > Posted via http://www.ruby-forum.com/.
    >
    >


    I meant rubyists. For example, the time_diff method at the very end can
    probably be rewritten

    def time_diff(t0, t1)
    day0 , hour0 , min0 , sec0 = t0.split(/\D+/)[2..-1].map { |n| n.to_i }
    day1 , hour1 , min1 , sec1 = t1.split(/\D+/)[2..-1].map { |n| n.to_i }

    sec0 += ( ( day0 * 24 + hour0 ) * 60 + min0 ) * 60
    sec1 += ( ( day1 * 24 + hour1 ) * 60 + min1 ) * 60
    sec = sec1-sec0

    sprintf '%02d:%02d:%02d' , sec/60/60 , sec/60%60 , sec%60
    end



    But I don't know for sure, without a bunch of tests to show it behaves the
    same way. I think the math at the end is probably right, but haven't had a
    math course in 2 years, would you rather trust correctness to my "probably
    right" or to your ironclad test suite? And when looking at it, I see we
    don't use year or month, so I wonder if it has bugs, and I'm just
    refactoring bugs, which makes it very difficult to get motivated. I might
    also be inclined to play with DateTime to see if it has this functionality
    within it, in which case the whole method could probably be replaced with 1
    line, but its not immediately obvious to me if it does, so it would be nice
    to have some tests that I can know how close I am getting.
    Josh Cheek, May 27, 2010
    #8
  9. @Josh,

    Sure, I actually want to use some of the time math ruby provides, and
    write appropriate tests for it. I just used some old crap as a place
    holder for now. I am in the process of writing tests and when I get to
    it, I will most certainly refactor.


    ;o)


    M

    Josh Cheek wrote:
    > On Thu, May 27, 2010 at 8:05 AM, Martin Hansen <> wrote:
    >
    >> > fear of breaking.

    >>
    >>

    >
    > I meant rubyists. For example, the time_diff method at the very end can
    > probably be rewritten
    >
    > def time_diff(t0, t1)
    > day0 , hour0 , min0 , sec0 = t0.split(/\D+/)[2..-1].map { |n| n.to_i }
    > day1 , hour1 , min1 , sec1 = t1.split(/\D+/)[2..-1].map { |n| n.to_i }
    >
    > sec0 += ( ( day0 * 24 + hour0 ) * 60 + min0 ) * 60
    > sec1 += ( ( day1 * 24 + hour1 ) * 60 + min1 ) * 60
    > sec = sec1-sec0
    >
    > sprintf '%02d:%02d:%02d' , sec/60/60 , sec/60%60 , sec%60
    > end
    >
    >
    >
    > But I don't know for sure, without a bunch of tests to show it behaves
    > the
    > same way. I think the math at the end is probably right, but haven't had
    > a
    > math course in 2 years, would you rather trust correctness to my
    > "probably
    > right" or to your ironclad test suite? And when looking at it, I see we
    > don't use year or month, so I wonder if it has bugs, and I'm just
    > refactoring bugs, which makes it very difficult to get motivated. I
    > might
    > also be inclined to play with DateTime to see if it has this
    > functionality
    > within it, in which case the whole method could probably be replaced
    > with 1
    > line, but its not immediately obvious to me if it does, so it would be
    > nice
    > to have some tests that I can know how close I am getting.


    --
    Posted via http://www.ruby-forum.com/.
    Martin Hansen, May 27, 2010
    #9
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. =?Utf-8?B?cm9kY2hhcg==?=

    code: organized

    =?Utf-8?B?cm9kY2hhcg==?=, Dec 22, 2005, in forum: ASP .Net
    Replies:
    0
    Views:
    401
    =?Utf-8?B?cm9kY2hhcg==?=
    Dec 22, 2005
  2. Ranjay
    Replies:
    8
    Views:
    285
    Michal Nazarewicz
    Aug 24, 2006
  3. senges
    Replies:
    0
    Views:
    297
    senges
    Mar 24, 2007
  4. Peng Yu
    Replies:
    1
    Views:
    389
    Jonathan Gardner
    Sep 29, 2009
  5. 123Jim
    Replies:
    0
    Views:
    307
    123Jim
    Mar 2, 2013
Loading...

Share This Page