[DataAccess] Add class for accessing physical filesystems

Review Request #67 — Created Feb. 23, 2013 and submitted

Information
pprkut
Lunr
master
Reviewers
lunr 
This adds a class (and a couple interfaces) to help with accessing physical filesystems in a similar fashion as the database access objects help accessing databases.

Note that we break the DI Pattern here a bit, as all the iterators used in the class are not injected. The (tiny) benefits we would get from injecting those do IMHO not outweigh the increased complexity we'd have in both testing and usage.
Unit Tests
  • 0
  • 0
  • 2
  • 1
  • 3
Description From Last Updated
dinos
dinos 
  1. 
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    not a 5.4?
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      What code in the file would be 5.4 specific?
      
 
      

      2. 



    2. 
 
 
      
 
      
  
 
      
 
      
  
      My bad. It's an interface :|
      
 
      

      3. 


 
    


    

 
    

    3. 






  3. 
 
    
  
  
    
  
 

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    this looks a bit weird, doesn't it? 
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      Maybe. Suggestions?
      
 
      

      2. 



    2. 
 
 
      
 
      
  
 
      
 
      
  
      I would say as a config value.
      
 
      

      3. 



    3. 
 
 
      
 
      
  
 
      
 
      
  
      I'm not sure that would be the best course of action. It's reused by a couple of other tests so maybe declaring it const or setting it in the constructor would be better, but I don't see what turning it into a config value would solve.
      
 
      

      4. 



    4. 
 
 
      
 
      
  
 
      
 
      
  
      This is the way that I 've seen that we are dealing with paths (with configs). As constant could be also fine with me. The point is having a single line maintenance, in case that it is needed. 
      
 
      

      5. 



    5. 
 
 
      
 
      
  
 
      
 
      
  
      Ah, I see. I've been working on eliminating all hardcoded path config variables from Lunr (see the L10n reviews for example). They were introduced as a means to easily inject a different path if you need to distinguish production from test environments (and they use different paths). That's still fine on application level, but in Lunr we now have better means to achieve the same.

In this case I would prefer having a protected attribute with that value rather than a constant.
      
 
      

      6. 


 
    


    

 
    

    4. 






  4. 
 
    
  
  
    
  
 

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    *finding
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    5. 




  5. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    6. 




  

 
















 



 

  

   




pprkut




   

   

  

 



 















 





 

  

   




dinos




   

   







  

 



 

  


   

   

    
 
dinos


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




pprkut




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  






 

  Status:
  

  Closed (submitted)

  
 




 
Change Summary:

 
Merged with master





 


 










  

 









     

    

   

  

  

   
   Loading...