Conversation
evelynnkaplan
left a comment
evelynnkaplan
left a comment
There was a problem hiding this comment.
A few small questions, look good otherwise.
| end | ||
| end | ||
|
|
||
| return xml_file |
There was a problem hiding this comment.
If xml_file remains nil, is that what we want to return?
|
|
||
| describe "find_xml_file" do | ||
| it "should return nil if there are no xml files" do | ||
| expect(find_xml_file('.')).must_be_nil |
There was a problem hiding this comment.
Could we assign the output of the method call to a variable and then expect that value of that variable to be nil?
| expect(find_xml_file('.')).must_be_nil | ||
| end | ||
|
|
||
| it "should return an xml file if it's in the folder" do |
| require 'minitest' | ||
| require 'minitest/autorun' | ||
| require 'minitest/reporters' | ||
| require 'minitest/skip_dsl' |
There was a problem hiding this comment.
Are all of these requires needed if we've already required minitest in the first require? As in, is autorun already imported when we require minitest?
| it "should return an xml file if it's in the folder" do | ||
| expect(find_xml_file('./test/test_data')).must_equal "example.xml" | ||
| end | ||
| end |
There was a problem hiding this comment.
Consider adding a test for a case in which there is more than one xml file.
|
|
||
| xml_file = nil | ||
|
|
||
| files.each do |filename| |
There was a problem hiding this comment.
I'm curious how the Dir.entries(folder) works. What order will the each loop evaluate the files in? This is for my own understanding of how this code works.
kanderson38
left a comment
kanderson38
left a comment
There was a problem hiding this comment.
Looks good! One comment for you.
|
|
||
| describe "find_xml_file" do | ||
| it "should return nil if there are no xml files" do | ||
| expect(find_xml_file('.')).must_be_nil |
There was a problem hiding this comment.
Should we pass an actual empty folder in to this test instead of the current directory?
| end | ||
| end | ||
|
|
||
| return xml_file |
There was a problem hiding this comment.
I would consider replacing the break in line 10 with this return xml_file to DRY up the code.
|
|
||
| Minitest::Reporters.use! Minitest::Reporters::SpecReporter.new | ||
|
|
||
| # Require_relative your lib files here! |
| def find_xml_file(folder) | ||
| files = Dir.entries(folder) | ||
|
|
||
| xml_file = nil |
There was a problem hiding this comment.
If no .xml in files, return will be nil.
| xml_file = nil | ||
|
|
||
| files.each do |filename| | ||
| if filename.match "\.xml$" |
There was a problem hiding this comment.
Dir['*.xml'] offers a convenient way to do this or Dir['**/*.xml'] in case of subdirectories
7 participants
This method finds the 1st xml file in the folder.