Re: Final iteration lost?

Robert Klemme <[email protected]> Thu, 15 Aug 2019 17:19:29 +0200
Newsgroups gmane.comp.lang.ruby.general
Message-ID <CAM9pMnOwoXQ51AmTCj9Q9LxJJQUY0whGcw_=38qMdgN5yv2HFA@mail.gmail.com>
On Thu, Aug 15, 2019 at 5:10 PM Robert Klemme <[email protected]>
wrote:

>
>
> On Thu, Aug 15, 2019 at 1:31 PM Leam Hall <[email protected]> wrote:
>
>> Here's the current iteration of that section.
>
>
> So it is not the final iteration? :P
>
>
>> The Job class
>>
> initialization takes some data and sets other values to nil. This is due
>> to some older jobs not having those data sections. The sections are
>> always in order.
>>
>> ###
>>
>> data = Hash.new
>> jobs_file = 'input/leamhall_jobs.txt'
>> File.foreach(jobs_file) { |line|
>>    line.chomp!.strip!
>>    next if line.empty?
>>    if line.end_with?(')')
>>      data.clear
>>      header_array  = line.split(',')
>>      title         = header_array.shift
>>      dates         = header_array.pop
>>      start_date, end_date = set_dates(dates)
>>      employer      = header_array.join(' ').strip!
>>      data = {  start_date: start_date, end_date: end_date,
>>                title: title, employer: employer }
>>      current_job   = Job.new(data)
>>      jobs << current_job
>>    elsif current_job.blurb.nil?
>>      current_job.blurb = line
>>    elsif current_job.tech.nil?
>>      current_job.tech  = line
>>    else
>>      current_job.extra = line
>>    end
>> }
>>
>
> I would definitively move parsing of the header line and creation of a new
> job into a separate function. Makes the line - section logic much more
> readable.
>

Sent too early.

Does this even work? current_job will be nil on each iteration because it
is not declared outside. Then this will happen:

irb(main):008:0> 5.times {|i| puts i; if rand(2) == 0 then puts "assign";
foo = Time.now end; p foo}
0
assign
2019-08-15 17:17:22 +0200
1
nil
2
assign
2019-08-15 17:17:22 +0200
3
nil
4
nil
=> 5

Also, you do not need data here. You always overwrite it with a new Hash
all the time. And you do not need to declare it outside the loop.

As an example

def Job.parse_header(line)
  header_array  = line.split(',')
  title         = header_array.shift
  dates         = header_array.pop
  start_date, end_date = set_dates(dates)
  employer      = header_array.join(' ').strip!

  data = {  start_date: start_date, end_date: end_date,
            title: title, employer: employer }

  current_job   = new(data)
end

Kind regards

robert

-- 
[guy, jim, charlie].each {|him| remember.him do |as, often| as.you_can -
without end}
http://blog.rubybestpractices.com/


Unsubscribe: <mailto:[email protected]?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-talk>