A sort of design question

Dear Ladies (??) and Gentlemen (okay we have plenty of those),

I wrote a script which needs a variable in order to proceed. This
variable is defined by the user otherwise the default location is
chosen. After a discussion I had on IRC (Ruby @ Freenode), I came up
with a few questions.

My script is structured like this:

class […]
optparse […]
execution_flow_control […]

I do the ‘control’ of this variable inside the class like this:

class Prog
attr_accessor :database
def initialize(database)
@database = database
end

[…]

def create_table(tableid)
if @database and File.exists?(@database) <— control
[…]

end

I was wondering if this is the right way actually do this control since,
if @database is not set, the flow will never reach that point, because
the class takes 1 argument (which is @database) and if it’s null, it
will come up with an error.

The script works fine, as is. I optimized it a bit, by removing and/or
optimizing some functions. But I’m curious, where is the right place for
doing this control which is necessary for execution?

Best Regards,

ps. Here is the entire script:
https://github.com/atmosx/morula/blob/master/morula

Panagiotis A.

On Wed, Jan 25, 2012 at 3:53 PM, Panagiotis A.
[email protected] wrote:

I do the ‘control’ of this variable inside the class like this:
[…]

end

I was wondering if this is the right way actually do this control since, if
@database is not set, the flow will never reach that point, because the class
takes 1 argument (which is @database) and if it’s null, it will come up with an
error.

I do not understand what you mean by “the flow will never reach that
point”: From what you have shown the constructor will happily accept
nil for parameter “database”. And if someone after that invokes
#create_table the code will reach just this point. Also, I am not
100% positive what you mean by “this control”.

The script works fine, as is. I optimized it a bit, by removing and/or
optimizing some functions. But I’m curious, where is the right place for doing
this control which is necessary for execution?

Considering what I see here and on github I’d say constructor of Prog
should check argument “database” for valid values and raise an
exception if nil is passed. Then, later on, the other methods can
rely on this value being valid. That’s basically what OO is all
about: you define the valid state of an instance and maintain it
throughout all methods.

Kind regards

robert

Hello,

On 25 Ιαν 2012, at 16:05 , Robert K. wrote:

execution_flow_control […]
def create_table(tableid)
#create_table the code will reach just this point. Also, I am not
100% positive what you mean by “this control”.

The script works fine, as is. I optimized it a bit, by removing and/or
optimizing some functions. But I’m curious, where is the right place for doing
this control which is necessary for execution?

Considering what I see here and on github I’d say constructor of Prog
should check argument “database” for valid values and raise an
exception if nil is passed. Then, later on, the other methods can
rely on this value being valid. That’s basically what OO is all
about: you define the valid state of an instance and maintain it
throughout all methods.

thanks, that was the answer I was looking for.

Kind regards

robert


remember.guy do |as, often| as.you_can - without end
http://blog.rubybestpractices.com/

Panagiotis A.

Panagliotis,

I think Robert K. is telling you to do something like this.

Abinoam Jr.

On Wed, Jan 25, 2012 at 2:05 PM, Panagiotis A.

Hello,

On 25 Ιαν 2012, at 23:34 , Abinoam Jr. wrote:

Panagliotis,

I think Robert K. is telling you to do something like this.

Abinoam Jr.

Thanks for that :slight_smile: I merged it. I didn’t had much time to look at the
code today so thanks for clarifying this in terms of code.

Best Regards

class Prog
end

Kind regards


Panagiotis A.

On Thu, Jan 26, 2012 at 1:35 AM, Panagiotis A.
[email protected] wrote:

On 25 2012, at 23:34 , Abinoam Jr. wrote:

I think Robert K. is telling you to do something like this.

Exactly!

Abinoam Jr.

Thanks for that :slight_smile: I merged it. I didn’t had much time to look at the code
today so thanks for clarifying this in terms of code.

Just one nitpick: in the method #create_table the begin rescue end
should have an “ensure” clause where the db.close is located. This
ensures that the db is closed no matter how the block is left. You
might also want to evaluate the actual exception’s error message.
Might be something else than “table exists”.

Kind regards

robert