[ruby-talk:8457] Re: string like istringstream (was: A bug inruby)

(Heavens, it's the third millenium!)

On Mon, 1 Jan 2001, Jean Michel wrote:

> Well, below is  my code. It is  a kind of 'minimal' solution:  I did not
> derive a different  class from string, and  I did not even  add a method
> as_file since .rewind  does the job. Also I have  yet implemented only a
> small subset of the ID3V2 standard, but which covers all the music files
> I have on  my hard disk. At  the bottom is a sample  application where I
> run over all  .mp3 files in a  directory, show the ID3v1  and ID3v2 tags
> and propose  to rename the  file to trak_no+title.mp3 (I  follow windows
> filename conventions, I work under  windows with djgpp). Any comments on
> how to shorten or rubify my code are welcome.
> 
> class String
>   def rewind
>     @current_pointer=0
>   end
>   def read(n)
>     @current_pointer+=n
>     if @current_pointer>=length
>        @current_pointer=length
>        nil

Wouldn't it be better, if the pointer is < length, to return what's
left of the string, even if pointer + n >= length?  Something like:

     def read(n)
       @pos += n
       res = self[@pos - n .. @pos - 1]
       @pos = length if @pos > length
       res
     end

In your version, if you start reading inside the string but go past
the end, you get nil (instead of the end of the string).  

(I like @pos better than @current_pointer, because it's shorter, still
readable, and similar to IO#pos.)

> Dir["*.mp3"].each{ |f| print "\n{",f,"}\n"

The #{} construct tends to look better:

   print "\n{#{f}}\n"

though admittedly in this case there are a lot of curly braces :-)

>   tag=open(f,"rb"){ |fl| print fl.read_ID3V1_tag; fl.read_ID3V2_tag}
>   print tag,"\n"
>   if tr=tag["TRCK"] then
>     newname=sprintf("%02d-%s.mp3",tr.data.to_i,tag["TIT2"].data.tr(
>        " :`\"\\/*?|","_;''--#_-"))
>     print "propose rename (y/n/q) to\n",newname

My main suggestion would be to think about some refactoring, such that
the following code (or something like it) would work:

   Dir["*.mp3"].each{ |f|
     print "\n{#{f}}\n"
     tag = ID3V2Tag.new
     tag.get_from_stream(open(f, "rb"))

As per my previous post, I find it sort of lop-sided to put the
knowledge of these tags in the String and IO classes.  It seems to me
that the design would be more balanced if the Tag class(es) could be
given a seekable stream (String or IO), and would know how to find a
tag from that stream by using seek, read, etc.


David

-- 
David Alan Black
home: [e-mail]
work: [e-mail]
Web:  http://pirate.shu.edu/~blackdav