Re: The relevant_lines package

[Petite Abeille <petite.abeille@...>]

On Jan 28, 2011, at 9:40 PM, Steve Litt wrote:

> I hope you enjoy it, and please be gentle when you tell me all the things I 
> did wrong with it. :-)

Nothing fundamentally wrong per say, but... just wondering... isn't it a bit overkill? :))

My 2¢...

>  function relevant_lines(tab)

Why a table as a parameter? What's wrong with explicit parameters? E.g.:

relevant_lines( aFile, is_relevant, tweak )

> if tab == nil then .. io.stderr:write... os.exit...

Look into  assert(), e.g.: 

assert( function() return type( tab ) == 'table' end(), 'Expected a table, got ' .. type( tab ) .. ' instead' )

http://www.lua.org/manual/5.1/manual.html#pdf-assert

> if .. elseif... elseif... else

A common idiom if you have many such simple condition is to use a table instead, e.g.:

-- define a mapping between type and function
aMap = { string = function( aValue ) return io.open( aValue , 'rb' ), ... }
-- lookup a function
aFunction = aMap[ type( aValue ) ]
-- invoke it
aFunction( aValue )

> elseif type(tab.file) == "userdata" then

Perhaps try io.type(obj) == 'file' instead

http://www.lua.org/manual/5.1/manual.html#pdf-io.type

>  if not tab.is_relevant then

Another handy idiom to provide default values:

tab.is_relevant = tab.is_relevant or function() ... end

> tab.this_line_text = ...

What about simply passing the value directly to your is_relevant() function? Is that shared table really adding anything? 

All in all, wouldn't it be clearer to simply say what you mean? E.g.:

for aLine in aFile:lines() do
  if Accept( aLine ) then 
    Transform( aLine )
  end
end

If that's too much typing, you can wrap it in a function or something, e.g.:

local function DWIM( aFile, aFilter, aTransformation )
  aFile = assert( aFile )
  aFilter = aFilter or function( aLine ) return true end
  aTransformation = aTransformation or function( aLine ) return aLine end

  return coroutine.wrap( function() for aLine in aFile:lines() do if aFilter( aLine ) then coroutine.yield( aTransformation( aLine ) ) end end end )
end