Is my code using the Ruby way?

[cmdjackryan]

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

 

[Helder Ribeiro]

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.

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

 

[cmdjackryan]

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

 

[Michael Fellinger]

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

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

^ manveru

 

[cmdjackryan]

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

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

-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

 

[ara.t.howard]

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).

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:atabase.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

 

[Robert Klemme]

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