[Gravity] Put to a CSV File

Review Request #299 — Created April 30, 2014 and submitted

Information
dinos
Lunr
dinostheo:PutCSVFile
Reviewers
lunr 
A function that writes a array list to a csv file
unit tests
  • 0
  • 0
  • 4
  • 0
  • 4
Description From Last Updated
leo
leo 
  1. 
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 

   
    
    
    




    
    The delimiter could be different than a comma, maybe use a slightly more generic (or delimiter-specific) warning message?
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      I mean comma separate values as CSV.
http://en.wikipedia.org/wiki/Comma-separated_values
      
 
      

      2. 



    2. 
 
 
      
 
      
  
 
      
 
      
  
      ok cool :)
      
 
      

      3. 


 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 





 

  

   




leo




   

   







  

 



 

  


   

   

    
 
leo


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    could you also add the method in the interface?
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 



 

  

   




dinos




   

   

  

 



 















 





 

  

   




leo




   

   







  

 



 

  


   

   

    
 
leo


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




leo




   

   





  

 



 

  


   

   

    
 
leo


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Just to comment, I tested the function in my code chain and it properly does what it's supposed to. :)
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




pprkut




   

   







  

 



 

  


   

   

    
 
pprkut


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    s/option/optional/ ?
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 






  3. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      src/Lunr/Gravity/Filesystem/Tests/PhysicalFilesystemAccessObjectCSVTest.php
      


       (Diff revision 2)


      
     
    
    
       
   
    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

       
  
    
 
    

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    while the mock works here, we don't really need it. I'd rather see an actually wrong array value here that provokes the error.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      I couldn't force an error. Invalid array values return 1 and not false. While it also creates an empty csv document.
      
 
      

      2. 



    2. 
 
 
      
 
      
  
 
      
 
      
  
      Having a mock only realy makes sense when you know what scenario you are mocking. I'd rather not have a mock here, but if we do go for the mock I want to mock exactly the scenario that would actually cause it and not some artificial guess.
Wrong array values should definitely be tested either way. If they don't cause a FALSE, then something else must. You could try an invalid file pointer.
      
 
      

      3. 



    3. 
 
 
      
 
      
  
 
      
 
      
  
      False is caused by invalid file pointer according to documentation. However, I don't know how to create one. 
I will write tests for the invalid array values.
      
 
      

      4. 



    4. 
 
 
      
 
      
  
 
      
 
      
  
      It's very likely that using our code one actually can't trigger that error. The only situation I could imagine is when something happens to the physical location after fopen succeeds but before we call fputcsv.
Something like permissions suddenly change, or the file is deleted or some such.

Either way, if it really is the file pointer at fault, then "Could not comma separate the values" is a totally misleading error message, because the error has nothing whatsoever to do with wrong values.
      
 
      

      5. 



    5. 
 
 
      
 
      
  
 
      
 
      
  
      I changed the message and added some array value tests. I will do try something of the following "Something like permissions suddenly change, or the file is deleted or some such." as soon as I find a way to do it on runtime. 
      
 
      

      6. 


 
    


    

 
    

    4. 




  4. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    5. 




  

 
















 



 

  

   




dinos




   

   

  

 



 















 



 

  

   




dinos




   

   

  

 



 















 



 

  

   




dinos




   

   

  

 



 















 





 

  

   




pprkut




   

   







  

 



 

  


   

   

    
 
pprkut


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      src/Lunr/Gravity/Filesystem/PhysicalFilesystemAccessObject.php
      


       (Diff revisions 4 - 5)


      
     
    
    
       
   
    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

       
  
    
 
    

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    fputcsv doesn't throw exceptions so the try/catch is superfluous
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 



 

  

   




dinos




   

   

  

 



 















 



 

  

   




dinos




   

   

  

 



 















 





 

  

   




pprkut




   

   







  

 



 

  


   

   

    
 
pprkut


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




dinos




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  






 

  Status:
  

  Closed (submitted)

  
 




 
Change Summary:

 
Merged into master





 


 










  

 









     

    

   

  

  

   
   Loading...