Splitomania - advice welcome

ruby-talk
1

Hi.

I’m writing a commitinfo script for CVS using ruby 1.6.7 on Debian
Woody. I’ve created a class that handles all the ugly stuff at a commit,
and the actual commitinfo-scripts uses this class to ease the CVS
interaction.

Now - I want to extract the editors of each file that gets commited.
This is done by parsing the $CVSHOME/CVS/fileattr file. I want to get
all editors as a Hash where the keys are the names of the editors, and
the values are the extra info for the editor. The file format of
$CVSHOME/CVS/fileattr is something like:

Fname_of_fileattribute_name=attribute_value[;attribute_name=attribute_value…]

the attribute containing the editors looks like:

_editors=name_of_editor>time+hostname+pathname[,name_of_editor>time+hostname+pathname…]

I’m somewhat new with ruby - and when I parse this file I end up with a
hole bunch of split(/this).last.split(/that/) and a lot of
if-statements. This is ugly and error-prone. I’m under the impression that this can be
done easy and elegant using ruby - so I’d like to know how you ruby
gurus would parse the file to get the editors as a Hash :slight_smile:

If you want to I can post my current code (it’s ugly).

//Anders

···

–
/**

  • Anders Engström, aengstrom@gnejs.net
  • Your mind is like an umbrella.
  • It doesn’t work unless you open it.
  • /Frank Zappa
    */
2

Anders Engström aengstrom@gnejs.net writes:

Hi.

I’m writing a commitinfo script for CVS using ruby 1.6.7 on Debian
Woody. I’ve created a class that handles all the ugly stuff at a commit,
and the actual commitinfo-scripts uses this class to ease the CVS
interaction.

Now - I want to extract the editors of each file that gets commited.
This is done by parsing the $CVSHOME/CVS/fileattr file. I want to get
all editors as a Hash where the keys are the names of the editors, and
the values are the extra info for the editor. The file format of
$CVSHOME/CVS/fileattr is something like:

Fname_of_fileattribute_name=attribute_value[;attribute_name=attribute_value…]

the attribute containing the editors looks like:

_editors=name_of_editor>time+hostname+pathname[,name_of_editor>time+hostname+pathname…]

I suspect the ugly code reflects the ugly file format you are
parsing. Here is my untested attempt.

editors = Hash.new

File.open(“fileattr”) { |line|
fname, attrs = line.split(“\t”, 2)
attrs.split(“;”).each { |attr|
next unless attr =~ /^_editors=(.+)/
$1.split(“,”).each { |editor|
name, stuff = editor.split(“>”)
editors[name] = stuff
}
}
}

If your ‘if’ statements are for catching file format errors, you might
replace them with a begin/rescue/end within the loop. But that can
hide bugs in your own code.

File.open(“fileattr”) { |line|
begin
fname, attrs = line.split(“\t”, 2)
attrs.split(“;”).each { |attr|
next unless attr =~ /^_editors=(.+)/
$1.split(“,”).each { |editor|
name, stuff = editor.split(“>”)
editors[name] = stuff
}
}
rescue
end
}

···

–
Don’t send mail to Rod_Smith@hole.lickey.com
The address is there for spammers to harvest.

3

Thanks for the input! This is my current code after applying your ideas:

def editors
	if @editors == nil
		@editors = Hash.new
		fileattr = "#{@cvswd}/CVS/fileattr"
		if File.exist?(fileattr)
			File.open(fileattr){ |file|
				line = nil 
				file.each_line { |l| line = l if l =~ /^F#{name}/ }
				if line != nil
					line.split("\t").last.split(";").each { |attpair|
						next unless attpair =~ /^_editors=(.+)/
						$1.split(",").each { |editor|
							name, stuff = editor.split(">")
							@editors[name] = stuff
						}
					}
				end
			}
		end
	end
	return @editors
end

the code is implemented in an accessor method (I don’t know if the above
is the prefered way to implement ‘lazy loading’ in Ruby - comments are
welcome :).

Any comments on the above code? Can I optimize it further?

//Anders

···

On Sat, 14 Dec 2002 05:48:19 +0900, Matt Armstrong matt@lickey.com thus spoketh: > Anders Engström aengstrom@gnejs.net writes:

Hi.

I’m writing a commitinfo script for CVS using ruby 1.6.7 on Debian
Woody. I’ve created a class that handles all the ugly stuff at a commit,
and the actual commitinfo-scripts uses this class to ease the CVS
interaction.

Now - I want to extract the editors of each file that gets commited.
This is done by parsing the $CVSHOME/CVS/fileattr file. I want to get
all editors as a Hash where the keys are the names of the editors, and
the values are the extra info for the editor. The file format of
$CVSHOME/CVS/fileattr is something like:

Fname_of_fileattribute_name=attribute_value[;attribute_name=attribute_value…]

the attribute containing the editors looks like:

_editors=name_of_editor>time+hostname+pathname[,name_of_editor>time+hostname+pathname…]

I suspect the ugly code reflects the ugly file format you are
parsing. Here is my untested attempt.

editors = Hash.new

File.open(“fileattr”) { |line|
fname, attrs = line.split(“\t”, 2)
attrs.split(“;”).each { |attr|
next unless attr =~ /^_editors=(.+)/
$1.split(“,”).each { |editor|
name, stuff = editor.split(“>”)
editors[name] = stuff
}
}
}

–
/**

  • Anders Engström, aengstrom@gnejs.net
  • Your mind is like an umbrella.
  • It doesn’t work unless you open it.
  • /Frank Zappa
    */
4

Anders Engström aengstrom@gnejs.net writes:

def editors
if @editors == nil
@editors = Hash.new
fileattr = “#{@cvswd}/CVS/fileattr”
if File.exist?(fileattr)
File.open(fileattr){ |file|
line = nil
file.each_line { |l| line = l if l =~ /^F#{name}/ }
if line != nil
line.split(“\t”).last.split(“;”).each { |attpair|
next unless attpair =~ /^_editors=(.+)/
$1.split(“,”).each { |editor|
name, stuff = editor.split(“>”)
@editors[name] = stuff
}
}
end
}
end
end
return @editors
end

It looks like you are using ‘name’ before initializing it.

Also, File.exit? doesn’t mean File.open will succeed. I would use
begin/rescue/end if you want to succeed when the file is not there.
E.g.

begin
rescue Errno::ENOENT
end

the code is implemented in an accessor method (I don’t know if the
above is the prefered way to implement ‘lazy loading’ in Ruby -
comments are welcome :).

Yes, I’ve done that. There is also the memoize package.
http://www.ruby-lang.org/raa/list.rhtml?id=34

5

def editors
if @editors == nil
@editors = Hash.new
fileattr = “#{@cvswd}/CVS/fileattr”
if File.exist?(fileattr)
File.open(fileattr){ |file|
line = nil
file.each_line { |l| line = l if l =~ /^F#{name}/ }
if line != nil
line.split(“\t”).last.split(“;”).each { |attpair|
next unless attpair =~ /^_editors=(.+)/
$1.split(“,”).each { |editor|
name, stuff = editor.split(“>”)
@editors[name] = stuff
}
}
end
}
end
end
return @editors
end

It looks like you are using ‘name’ before initializing it.

Ah… that is supposed to be “@name” :slight_smile: Thanks for pointing it out!

Also, File.exit? doesn’t mean File.open will succeed. I would use
begin/rescue/end if you want to succeed when the file is not there.
E.g.

begin
rescue Errno::ENOENT
end

If the file can not be opened - then something is seriously wrong. I
handle exceptions in the main program flow.

the code is implemented in an accessor method (I don’t know if the
above is the prefered way to implement ‘lazy loading’ in Ruby -
comments are welcome :).

Yes, I’ve done that. There is also the memoize package.
http://www.ruby-lang.org/raa/list.rhtml?id=34

Thanks for the URL. That looks nice! No reason to use it in a CVS
commit-script though (because the script is extremly short-lived). But
I’ll keep the link for future projects :slight_smile:

//Anders

···

On Tue, 17 Dec 2002 03:01:41 +0900, Matt Armstrong matt@lickey.com thus spoketh: > Anders Engström aengstrom@gnejs.net writes:

–
/**

  • Anders Engström, aengstrom@gnejs.net
  • Your mind is like an umbrella.
  • It doesn’t work unless you open it.
  • /Frank Zappa
    */