Is my code using the Ruby way?

As I’m a beginner in Ruby, and just (re)started working on real code,
instead of working through various tutorials, I’m asking myself: Is that
the Ruby way?

Snippet:

unless File.exists?(‘data/finance.db’) == true
setup = Database.new
setup.new_database
else
puts ‘Database already exists!’
end

The code is tested and working as expected (Database is my own class,
which creates the database in the first place), and I don’t think it
could be (much) shorter.

-Phill

On Jan 29, 8:52 pm, [email protected] wrote:

 puts 'Database already exists!'

end

The code is tested and working as expected (Database is my own class,
which creates the database in the first place), and I don’t think it
could be (much) shorter.

I’m a ruby-nuby as well, but for one thing I think you don’t need that
“== true” after the condition. If the method ‘exists?’ returns true,
it is already taken as the value of the expression ‘unless’ needs to
evaluate, so there’s no need for the comparison.

Cheers,

Helder

Also, I’d put the ‘else’ on the same level of indentation as the unless.
As
Helder wrote, you don’t need the == true bit, Ruby’s all about natural
code.

Tamreen Khan wrote:

Also, I’d put the ‘else’ on the same level of indentation as the unless. As
Helder wrote, you don’t need the == true bit, Ruby’s all about natural
code.

Yeah, I did the former already (I like it better that way). And omitting
the ==true part makes the code more readable.

Thanks for the feedback, folks.

-Phill

On 1/30/07, [email protected] [email protected]
wrote:

 puts 'Database already exists!'

end

class Database
def self.create file = ‘data/finance.db’
if File.exist?(file)
puts ‘Database already exists!’
else
new file
end
end

Database.create

The code is tested and working as expected (Database is my own class,
which creates the database in the first place), and I don’t think it
could be (much) shorter.

This is just to show that Database itself should take responsibility
over creation/checking. It’s just a small example but should give you
an idea.
Tell, don’t do :slight_smile:

^ manveru

On Tue, 30 Jan 2007 [email protected] wrote:

already existing, and if not, to have a set of SQLite statements create it,
which I probably place in their own class, as the SQL can change, and this
would provide easier maintenance, and a better re-usability of the code,
too).

hopefully you realize this code contains a race condition. if you use a
two
step process to check that the database is created and, if not, to
create it -
you risk that another process might create it in between those two
steps. a
cleaner way to do it is to use execptions:

harp:~ > cat a.rb
require ‘sqlite’

class Database
SCHEMA = <<-SCHEMA
create table t(
id int,
data string
);
SCHEMA

 def initialize path
   @path = path
   @db = SQLite::Database.new @path
   setup
 end

 def setup
   @db.execute SCHEMA rescue nil
 ensure
   validate_schema
 end

 def validate_schema
   @db.execute 'select * from t limit 1'
 end

end

Database.new ‘db’

here, the db is always created and initialized with the SCHEMA.
normally
trying to create the table twice would throw an error, which is ingored.
because ignoring this error might mask a real failure to set the db a
simplistic method, in this case, is used to make sure the database looks
like
it’s supposed to. note that this whole thing is based on the knowledge
that
sqlite uses it’s own locking to ensure atmoicity.

in any case, the pattern of

“try it and recover if it blows up”

is often a good solution where testing a condition and then acting on it
cannot be done in one step to avoid a race condition.

btw. sqlite and ruby are a good combination - i use them heavily in my
own
work.

kind regards.

-a

Michael F. wrote:

This is just to show that Database itself should take responsibility
over creation/checking. It’s just a small example but should give you
an idea.
Tell, don’t do :slight_smile:

Ah, yes, that makes sense. I had to adapt your example to my needs, but
that is only a minor thing (I only use File to check if my SQLite
database is already existing, and if not, to have a set of SQLite
statements create it, which I probably place in their own class, as the
SQL can change, and this would provide easier maintenance, and a better
re-usability of the code, too).

Although this raises another question for me: Is it better to keep
functionality (i.e. everything that handles a database) into a class, or
to group functions into a class? The former keeps everything neatly in
one place, while the latter makes inheritance easier.

I lean towards a case-by-case basis, depending on the curcumstances of a
project, but to keep to stick with one option or the other (Phill’s
PLOS, so to speak :wink:

-Phill

Over by the window, lie the raiment and the weapons
That we need to take into this world today
Armoured by opinion, with statistic and schoolboy’s charm
We take our place amongst the rank and file

Skyclad - A Survival Campaign

On 1/29/07, [email protected] [email protected] wrote:

require ‘sqlite’
@path = path
def validate_schema
like

btw. sqlite and ruby are a good combination - i use them heavily in my
own
work.

Just a general thank you for this detailed explanation - now I
understand
better how to avoid race conditions (and I’m much more certain of what
exactly they are now).

On 30.01.2007 01:37, [email protected] wrote:

Tamreen Khan wrote:

Also, I’d put the ‘else’ on the same level of indentation as the
unless. As
Helder wrote, you don’t need the == true bit, Ruby’s all about natural
code.

Yeah, I did the former already (I like it better that way). And omitting
the ==true part makes the code more readable.

It actually also makes to code more correct. In Ruby only nil and false
are treated as false and everything else is true. So “if expr” and “if
expr == true” are likely to behave very differently.

With regard to your original piece of code, since you are using both
branches (true and false) I would prefer to use “if” as it makes things
a bit more readable IMHO:

if File.exists? ‘data/finance.db’
puts ‘Database already exists!’
else
setup = Database.new
setup.new_database
end

Kind regards

robert