Scheduled file sorting with Ruby












5














I have a script running as an hourly cron job. It finds all screenshots on the Desktop and moves them to directories sorted by year/month/day.



On MacOS, screenshots are automatically saved to the desktop with the following filename structure:



"/Users/<USER>/Desktop/Screen Shot 2019-01-02 at 11.56.42 AM.png"



require 'fileutils'

class Screenshot
attr_reader :filepath

def initialize(filepath)
@filepath = filepath
end

def dir_exists?
File.exists?(destination) || FileUtils.mkdir_p(destination)
end

def date
@date ||= filepath.match(screenshot_regex)
end

def move
FileUtils.mv(filepath, destination) if dir_exists?
end

def base_path
"/Users/home/Pictures/Screenshots"
end

def destination
"#{base_path}/#{date[:year]}/#{date[:month]}/#{date[:day]}"
end

def screenshot_regex
/Shot (?<year>(.*))-(?<month>(.*))-(?<day>(.*)) at/
end
end

class Screenshots
attr_reader :directory
def initialize(directory)
@directory = directory
end

def filepaths
Dir.glob("#{directory}/Screen Shot*.png")
end

def files
filepaths.map{|i| Screenshot.new(i)}
end

def move_all
files.each(&:move)
end
end

Screenshots.new("/Users/home/Desktop").move_all


I'm sure there's a less messy approach here and I'd love to hear any criticism.










share|improve this question









New contributor




alex is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.

























    5














    I have a script running as an hourly cron job. It finds all screenshots on the Desktop and moves them to directories sorted by year/month/day.



    On MacOS, screenshots are automatically saved to the desktop with the following filename structure:



    "/Users/<USER>/Desktop/Screen Shot 2019-01-02 at 11.56.42 AM.png"



    require 'fileutils'

    class Screenshot
    attr_reader :filepath

    def initialize(filepath)
    @filepath = filepath
    end

    def dir_exists?
    File.exists?(destination) || FileUtils.mkdir_p(destination)
    end

    def date
    @date ||= filepath.match(screenshot_regex)
    end

    def move
    FileUtils.mv(filepath, destination) if dir_exists?
    end

    def base_path
    "/Users/home/Pictures/Screenshots"
    end

    def destination
    "#{base_path}/#{date[:year]}/#{date[:month]}/#{date[:day]}"
    end

    def screenshot_regex
    /Shot (?<year>(.*))-(?<month>(.*))-(?<day>(.*)) at/
    end
    end

    class Screenshots
    attr_reader :directory
    def initialize(directory)
    @directory = directory
    end

    def filepaths
    Dir.glob("#{directory}/Screen Shot*.png")
    end

    def files
    filepaths.map{|i| Screenshot.new(i)}
    end

    def move_all
    files.each(&:move)
    end
    end

    Screenshots.new("/Users/home/Desktop").move_all


    I'm sure there's a less messy approach here and I'd love to hear any criticism.










    share|improve this question









    New contributor




    alex is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
    Check out our Code of Conduct.























      5












      5








      5







      I have a script running as an hourly cron job. It finds all screenshots on the Desktop and moves them to directories sorted by year/month/day.



      On MacOS, screenshots are automatically saved to the desktop with the following filename structure:



      "/Users/<USER>/Desktop/Screen Shot 2019-01-02 at 11.56.42 AM.png"



      require 'fileutils'

      class Screenshot
      attr_reader :filepath

      def initialize(filepath)
      @filepath = filepath
      end

      def dir_exists?
      File.exists?(destination) || FileUtils.mkdir_p(destination)
      end

      def date
      @date ||= filepath.match(screenshot_regex)
      end

      def move
      FileUtils.mv(filepath, destination) if dir_exists?
      end

      def base_path
      "/Users/home/Pictures/Screenshots"
      end

      def destination
      "#{base_path}/#{date[:year]}/#{date[:month]}/#{date[:day]}"
      end

      def screenshot_regex
      /Shot (?<year>(.*))-(?<month>(.*))-(?<day>(.*)) at/
      end
      end

      class Screenshots
      attr_reader :directory
      def initialize(directory)
      @directory = directory
      end

      def filepaths
      Dir.glob("#{directory}/Screen Shot*.png")
      end

      def files
      filepaths.map{|i| Screenshot.new(i)}
      end

      def move_all
      files.each(&:move)
      end
      end

      Screenshots.new("/Users/home/Desktop").move_all


      I'm sure there's a less messy approach here and I'd love to hear any criticism.










      share|improve this question









      New contributor




      alex is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
      Check out our Code of Conduct.











      I have a script running as an hourly cron job. It finds all screenshots on the Desktop and moves them to directories sorted by year/month/day.



      On MacOS, screenshots are automatically saved to the desktop with the following filename structure:



      "/Users/<USER>/Desktop/Screen Shot 2019-01-02 at 11.56.42 AM.png"



      require 'fileutils'

      class Screenshot
      attr_reader :filepath

      def initialize(filepath)
      @filepath = filepath
      end

      def dir_exists?
      File.exists?(destination) || FileUtils.mkdir_p(destination)
      end

      def date
      @date ||= filepath.match(screenshot_regex)
      end

      def move
      FileUtils.mv(filepath, destination) if dir_exists?
      end

      def base_path
      "/Users/home/Pictures/Screenshots"
      end

      def destination
      "#{base_path}/#{date[:year]}/#{date[:month]}/#{date[:day]}"
      end

      def screenshot_regex
      /Shot (?<year>(.*))-(?<month>(.*))-(?<day>(.*)) at/
      end
      end

      class Screenshots
      attr_reader :directory
      def initialize(directory)
      @directory = directory
      end

      def filepaths
      Dir.glob("#{directory}/Screen Shot*.png")
      end

      def files
      filepaths.map{|i| Screenshot.new(i)}
      end

      def move_all
      files.each(&:move)
      end
      end

      Screenshots.new("/Users/home/Desktop").move_all


      I'm sure there's a less messy approach here and I'd love to hear any criticism.







      object-oriented ruby datetime file-system scheduled-tasks






      share|improve this question









      New contributor




      alex is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
      Check out our Code of Conduct.











      share|improve this question









      New contributor




      alex is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
      Check out our Code of Conduct.









      share|improve this question




      share|improve this question








      edited Jan 2 at 20:10









      200_success

      128k15152413




      128k15152413






      New contributor




      alex is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
      Check out our Code of Conduct.









      asked Jan 2 at 19:43









      alex

      263




      263




      New contributor




      alex is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
      Check out our Code of Conduct.





      New contributor





      alex is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
      Check out our Code of Conduct.






      alex is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
      Check out our Code of Conduct.






















          1 Answer
          1






          active

          oldest

          votes


















          2














          Your program is over-engineered in a way that makes it hard to decipher. Because every method is one line long, it's like reading a poem whose lines have been scrambled. You would be much better off writing a simple function, like this:



          require 'fileutils'

          def move_screenshots(src_dir, dest_tree)
          re = /^Screen Shot (?<year>d{4})-(?<month>d{2})-(?<day>d{2}) at/
          Dir.foreach(src_dir) do |filename|
          if date = re.match(filename)
          dest_dir = "#{dest_tree}/#{date[:year]}/#{date[:month]}/#{date[:day]}"
          FileUtils.mkdir_p(dest_dir)
          FileUtils.mv("#{src_dir}/#{filename}", dest_dir)
          end
          end
          end

          move_screenshots("/Users/home/Desktop", "/Users/home/Pictures/Screenshots")


          I wouldn't bother with Dir#glob, since it's a bit redundant with the regex. Note that screenshots aren't necessarily in PNG format: the image format can be configured using defaults write com.apple.screencapture type …. I also wouldn't bother testing File#exists? before calling FileUtils#mkdir_p, since mkdir_p implicitly performs that check anyway.



          Instead of an hourly cron job, consider creating a Folder Action Script that is triggered instantly when a file is added to the folder.






          share|improve this answer





















          • I started off with something similar but broke it apart into those classes mostly for fun, but also with the intent to eventually revisit and expand it to target other common filetypes that might end up on my desktop. Wanted to use these as a starting point for that. Awesome suggestion on the folder action scripts. I had no idea.
            – alex
            2 days ago










          • Imagining for a second that this is part of a larger program, would this be an acceptable way to interact with a collection of object x that all need action y performed on them (separate class to represent the collection, mapping the seed data into a group of x instances, then calling y on each from within the collection obj)? Or would I still be overcomplicating it?
            – alex
            2 days ago










          • If your program needs to be extensible, or if the classes are polymorphic in more than one method, then OOP might be worthwhile. Otherwise, you are probably overcomplicating it. Post a separate question with a specific use case, if you are unsure.
            – 200_success
            2 days ago











          Your Answer





          StackExchange.ifUsing("editor", function () {
          return StackExchange.using("mathjaxEditing", function () {
          StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
          StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
          });
          });
          }, "mathjax-editing");

          StackExchange.ifUsing("editor", function () {
          StackExchange.using("externalEditor", function () {
          StackExchange.using("snippets", function () {
          StackExchange.snippets.init();
          });
          });
          }, "code-snippets");

          StackExchange.ready(function() {
          var channelOptions = {
          tags: "".split(" "),
          id: "196"
          };
          initTagRenderer("".split(" "), "".split(" "), channelOptions);

          StackExchange.using("externalEditor", function() {
          // Have to fire editor after snippets, if snippets enabled
          if (StackExchange.settings.snippets.snippetsEnabled) {
          StackExchange.using("snippets", function() {
          createEditor();
          });
          }
          else {
          createEditor();
          }
          });

          function createEditor() {
          StackExchange.prepareEditor({
          heartbeatType: 'answer',
          autoActivateHeartbeat: false,
          convertImagesToLinks: false,
          noModals: true,
          showLowRepImageUploadWarning: true,
          reputationToPostImages: null,
          bindNavPrevention: true,
          postfix: "",
          imageUploader: {
          brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
          contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
          allowUrls: true
          },
          onDemand: true,
          discardSelector: ".discard-answer"
          ,immediatelyShowMarkdownHelp:true
          });


          }
          });






          alex is a new contributor. Be nice, and check out our Code of Conduct.










          draft saved

          draft discarded


















          StackExchange.ready(
          function () {
          StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f210760%2fscheduled-file-sorting-with-ruby%23new-answer', 'question_page');
          }
          );

          Post as a guest















          Required, but never shown

























          1 Answer
          1






          active

          oldest

          votes








          1 Answer
          1






          active

          oldest

          votes









          active

          oldest

          votes






          active

          oldest

          votes









          2














          Your program is over-engineered in a way that makes it hard to decipher. Because every method is one line long, it's like reading a poem whose lines have been scrambled. You would be much better off writing a simple function, like this:



          require 'fileutils'

          def move_screenshots(src_dir, dest_tree)
          re = /^Screen Shot (?<year>d{4})-(?<month>d{2})-(?<day>d{2}) at/
          Dir.foreach(src_dir) do |filename|
          if date = re.match(filename)
          dest_dir = "#{dest_tree}/#{date[:year]}/#{date[:month]}/#{date[:day]}"
          FileUtils.mkdir_p(dest_dir)
          FileUtils.mv("#{src_dir}/#{filename}", dest_dir)
          end
          end
          end

          move_screenshots("/Users/home/Desktop", "/Users/home/Pictures/Screenshots")


          I wouldn't bother with Dir#glob, since it's a bit redundant with the regex. Note that screenshots aren't necessarily in PNG format: the image format can be configured using defaults write com.apple.screencapture type …. I also wouldn't bother testing File#exists? before calling FileUtils#mkdir_p, since mkdir_p implicitly performs that check anyway.



          Instead of an hourly cron job, consider creating a Folder Action Script that is triggered instantly when a file is added to the folder.






          share|improve this answer





















          • I started off with something similar but broke it apart into those classes mostly for fun, but also with the intent to eventually revisit and expand it to target other common filetypes that might end up on my desktop. Wanted to use these as a starting point for that. Awesome suggestion on the folder action scripts. I had no idea.
            – alex
            2 days ago










          • Imagining for a second that this is part of a larger program, would this be an acceptable way to interact with a collection of object x that all need action y performed on them (separate class to represent the collection, mapping the seed data into a group of x instances, then calling y on each from within the collection obj)? Or would I still be overcomplicating it?
            – alex
            2 days ago










          • If your program needs to be extensible, or if the classes are polymorphic in more than one method, then OOP might be worthwhile. Otherwise, you are probably overcomplicating it. Post a separate question with a specific use case, if you are unsure.
            – 200_success
            2 days ago
















          2














          Your program is over-engineered in a way that makes it hard to decipher. Because every method is one line long, it's like reading a poem whose lines have been scrambled. You would be much better off writing a simple function, like this:



          require 'fileutils'

          def move_screenshots(src_dir, dest_tree)
          re = /^Screen Shot (?<year>d{4})-(?<month>d{2})-(?<day>d{2}) at/
          Dir.foreach(src_dir) do |filename|
          if date = re.match(filename)
          dest_dir = "#{dest_tree}/#{date[:year]}/#{date[:month]}/#{date[:day]}"
          FileUtils.mkdir_p(dest_dir)
          FileUtils.mv("#{src_dir}/#{filename}", dest_dir)
          end
          end
          end

          move_screenshots("/Users/home/Desktop", "/Users/home/Pictures/Screenshots")


          I wouldn't bother with Dir#glob, since it's a bit redundant with the regex. Note that screenshots aren't necessarily in PNG format: the image format can be configured using defaults write com.apple.screencapture type …. I also wouldn't bother testing File#exists? before calling FileUtils#mkdir_p, since mkdir_p implicitly performs that check anyway.



          Instead of an hourly cron job, consider creating a Folder Action Script that is triggered instantly when a file is added to the folder.






          share|improve this answer





















          • I started off with something similar but broke it apart into those classes mostly for fun, but also with the intent to eventually revisit and expand it to target other common filetypes that might end up on my desktop. Wanted to use these as a starting point for that. Awesome suggestion on the folder action scripts. I had no idea.
            – alex
            2 days ago










          • Imagining for a second that this is part of a larger program, would this be an acceptable way to interact with a collection of object x that all need action y performed on them (separate class to represent the collection, mapping the seed data into a group of x instances, then calling y on each from within the collection obj)? Or would I still be overcomplicating it?
            – alex
            2 days ago










          • If your program needs to be extensible, or if the classes are polymorphic in more than one method, then OOP might be worthwhile. Otherwise, you are probably overcomplicating it. Post a separate question with a specific use case, if you are unsure.
            – 200_success
            2 days ago














          2












          2








          2






          Your program is over-engineered in a way that makes it hard to decipher. Because every method is one line long, it's like reading a poem whose lines have been scrambled. You would be much better off writing a simple function, like this:



          require 'fileutils'

          def move_screenshots(src_dir, dest_tree)
          re = /^Screen Shot (?<year>d{4})-(?<month>d{2})-(?<day>d{2}) at/
          Dir.foreach(src_dir) do |filename|
          if date = re.match(filename)
          dest_dir = "#{dest_tree}/#{date[:year]}/#{date[:month]}/#{date[:day]}"
          FileUtils.mkdir_p(dest_dir)
          FileUtils.mv("#{src_dir}/#{filename}", dest_dir)
          end
          end
          end

          move_screenshots("/Users/home/Desktop", "/Users/home/Pictures/Screenshots")


          I wouldn't bother with Dir#glob, since it's a bit redundant with the regex. Note that screenshots aren't necessarily in PNG format: the image format can be configured using defaults write com.apple.screencapture type …. I also wouldn't bother testing File#exists? before calling FileUtils#mkdir_p, since mkdir_p implicitly performs that check anyway.



          Instead of an hourly cron job, consider creating a Folder Action Script that is triggered instantly when a file is added to the folder.






          share|improve this answer












          Your program is over-engineered in a way that makes it hard to decipher. Because every method is one line long, it's like reading a poem whose lines have been scrambled. You would be much better off writing a simple function, like this:



          require 'fileutils'

          def move_screenshots(src_dir, dest_tree)
          re = /^Screen Shot (?<year>d{4})-(?<month>d{2})-(?<day>d{2}) at/
          Dir.foreach(src_dir) do |filename|
          if date = re.match(filename)
          dest_dir = "#{dest_tree}/#{date[:year]}/#{date[:month]}/#{date[:day]}"
          FileUtils.mkdir_p(dest_dir)
          FileUtils.mv("#{src_dir}/#{filename}", dest_dir)
          end
          end
          end

          move_screenshots("/Users/home/Desktop", "/Users/home/Pictures/Screenshots")


          I wouldn't bother with Dir#glob, since it's a bit redundant with the regex. Note that screenshots aren't necessarily in PNG format: the image format can be configured using defaults write com.apple.screencapture type …. I also wouldn't bother testing File#exists? before calling FileUtils#mkdir_p, since mkdir_p implicitly performs that check anyway.



          Instead of an hourly cron job, consider creating a Folder Action Script that is triggered instantly when a file is added to the folder.







          share|improve this answer












          share|improve this answer



          share|improve this answer










          answered Jan 2 at 20:50









          200_success

          128k15152413




          128k15152413












          • I started off with something similar but broke it apart into those classes mostly for fun, but also with the intent to eventually revisit and expand it to target other common filetypes that might end up on my desktop. Wanted to use these as a starting point for that. Awesome suggestion on the folder action scripts. I had no idea.
            – alex
            2 days ago










          • Imagining for a second that this is part of a larger program, would this be an acceptable way to interact with a collection of object x that all need action y performed on them (separate class to represent the collection, mapping the seed data into a group of x instances, then calling y on each from within the collection obj)? Or would I still be overcomplicating it?
            – alex
            2 days ago










          • If your program needs to be extensible, or if the classes are polymorphic in more than one method, then OOP might be worthwhile. Otherwise, you are probably overcomplicating it. Post a separate question with a specific use case, if you are unsure.
            – 200_success
            2 days ago


















          • I started off with something similar but broke it apart into those classes mostly for fun, but also with the intent to eventually revisit and expand it to target other common filetypes that might end up on my desktop. Wanted to use these as a starting point for that. Awesome suggestion on the folder action scripts. I had no idea.
            – alex
            2 days ago










          • Imagining for a second that this is part of a larger program, would this be an acceptable way to interact with a collection of object x that all need action y performed on them (separate class to represent the collection, mapping the seed data into a group of x instances, then calling y on each from within the collection obj)? Or would I still be overcomplicating it?
            – alex
            2 days ago










          • If your program needs to be extensible, or if the classes are polymorphic in more than one method, then OOP might be worthwhile. Otherwise, you are probably overcomplicating it. Post a separate question with a specific use case, if you are unsure.
            – 200_success
            2 days ago
















          I started off with something similar but broke it apart into those classes mostly for fun, but also with the intent to eventually revisit and expand it to target other common filetypes that might end up on my desktop. Wanted to use these as a starting point for that. Awesome suggestion on the folder action scripts. I had no idea.
          – alex
          2 days ago




          I started off with something similar but broke it apart into those classes mostly for fun, but also with the intent to eventually revisit and expand it to target other common filetypes that might end up on my desktop. Wanted to use these as a starting point for that. Awesome suggestion on the folder action scripts. I had no idea.
          – alex
          2 days ago












          Imagining for a second that this is part of a larger program, would this be an acceptable way to interact with a collection of object x that all need action y performed on them (separate class to represent the collection, mapping the seed data into a group of x instances, then calling y on each from within the collection obj)? Or would I still be overcomplicating it?
          – alex
          2 days ago




          Imagining for a second that this is part of a larger program, would this be an acceptable way to interact with a collection of object x that all need action y performed on them (separate class to represent the collection, mapping the seed data into a group of x instances, then calling y on each from within the collection obj)? Or would I still be overcomplicating it?
          – alex
          2 days ago












          If your program needs to be extensible, or if the classes are polymorphic in more than one method, then OOP might be worthwhile. Otherwise, you are probably overcomplicating it. Post a separate question with a specific use case, if you are unsure.
          – 200_success
          2 days ago




          If your program needs to be extensible, or if the classes are polymorphic in more than one method, then OOP might be worthwhile. Otherwise, you are probably overcomplicating it. Post a separate question with a specific use case, if you are unsure.
          – 200_success
          2 days ago










          alex is a new contributor. Be nice, and check out our Code of Conduct.










          draft saved

          draft discarded


















          alex is a new contributor. Be nice, and check out our Code of Conduct.













          alex is a new contributor. Be nice, and check out our Code of Conduct.












          alex is a new contributor. Be nice, and check out our Code of Conduct.
















          Thanks for contributing an answer to Code Review Stack Exchange!


          • Please be sure to answer the question. Provide details and share your research!

          But avoid



          • Asking for help, clarification, or responding to other answers.

          • Making statements based on opinion; back them up with references or personal experience.


          Use MathJax to format equations. MathJax reference.


          To learn more, see our tips on writing great answers.





          Some of your past answers have not been well-received, and you're in danger of being blocked from answering.


          Please pay close attention to the following guidance:


          • Please be sure to answer the question. Provide details and share your research!

          But avoid



          • Asking for help, clarification, or responding to other answers.

          • Making statements based on opinion; back them up with references or personal experience.


          To learn more, see our tips on writing great answers.




          draft saved


          draft discarded














          StackExchange.ready(
          function () {
          StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f210760%2fscheduled-file-sorting-with-ruby%23new-answer', 'question_page');
          }
          );

          Post as a guest















          Required, but never shown





















































          Required, but never shown














          Required, but never shown












          Required, but never shown







          Required, but never shown

































          Required, but never shown














          Required, but never shown












          Required, but never shown







          Required, but never shown







          Popular posts from this blog

          Сан-Квентин

          8-я гвардейская общевойсковая армия

          Алькесар