1
\$\begingroup\$

I am currently learning to code in ruby. I have created a small program that iterates through a directory of video files and opens one at random. The program is fully functioning (with the odd issue). The aim of my question is to hopefully gain some insight into how I can better structure the code, or simplify the process - I think this will really aid me in my learning!

The random selection method I have created, creates 2 random numbers and converts them to a string:

def random_num_generator
  first_num = rand(2..9) #NOTE: generates a random number that is firt name and last name. s
  second_num = rand(1..25)
  second_num < 10 ? "0#{first_num}0#{second_num}" : "0#{first_num}#{second_num}"
  #this returns value that, for example is 0101, relative to whether or not there is a requirement for the additional "0"
  #value is equal to a string
end

The second part of the program iterates through the directory where the video files are stored. As all the file names are written as "0101.mkv" for example, the iteration will then stop and open a file if it is equal to the number generated in the "random_num_generator" method.

 def episode_picker
  Dir.foreach("/GitHub/videos") do |x|
    next if x == "." or x == ".."
    if x == "#{random_num_generator}.mkv"
      system %{open "/GitHub/videos/#{x}"}
    elsif x == "#{random_num_generator}.mp4"
      puts "You are watching Season: #{x[0..1]} Episode: #{x[2..3]}!"
      system %{open "/GitHub/videos/#{x}"}
    elsif x == "#{random_num_generator}.avi"
      puts "You are watching Season: #{x[0..1]} Episode: #{x[2..3]}!"
      system %{open "/GitHub/videos/#{x}"}

    end
  end
end

Any advice is greatly appreciated and a big thanks in advance!

\$\endgroup\$
  • \$\begingroup\$ Please state what your question does in the title, not what you are looking for. \$\endgroup\$ – FreezePhoenix Sep 24 '18 at 12:38
  • \$\begingroup\$ @FreezePhoenix thanks for the response, I have amended the title in accordance. \$\endgroup\$ – Ryan Barker Sep 24 '18 at 12:54
0
\$\begingroup\$

I'll try to help you on how you could improve your code here. Keep in mind that I have no idea what is your current level in programming in general. You state that you started learning ruby, but you don't tell us about other languages.

Random number generator

This method is fine, but what you want is to avoid having to comment every line. Comments are usually needed when the code is not clear. We'll try to make its meaning more obvious.

Let's try first to find better names here. It can seem like a little detail, but it is pretty important if you want to be able to read it in the future.

Names are very hard to find, you want names that give the meaning of what you are doing, and not how you are doing it.

For instance first_num is not as good as season, or season_number, because it gives less information.

This is true for functions names too. The result of your random_number_generator is a string that you will use as a file name. Based on the function's name, you would expect to get an object that generates numbers.

Let's rename it to random_file_name for instance.

This would make us write this function

def random_file_name
  season = rand(2..9)
  episode = rand(1..25)
  "#{two_chars(season)}#{two_chars(episode)}"
end

Did I mention it would be good to extract the logic of turning a number into a 2 characters string?

Let's create this two_chars function

def two_chars(number)
  number.to_s.rjust(2, '0')
end

Calling two_chars(1) will take 1, convert it to a string with to_s and fill the string with 0s until the string's size is 2. This way we won't need this test second_num < 10.

Episode Picker

In this function you are looping through all files in your folder; there is a more efficient way of doing it.

Plus you are calling random_num_generator several times in the same loop, and it will generate different filenames. This is probably not what you want: it could not find any file, or find several different files.

What you want is something looking like

def pick_episode
  file = file_with_name(random_file_name)
  puts "You are watching Season: #{filename[0..1]} Episode: #{filename[2..3]}!"

  system %{open "/GitHub/videos/#{file}"}
end

I changed the function's name to pick_episode since that what it does. What we do is find a random file, display that we are watching it, and open it.

Since you ask what is in this file_with_name function, here is how you could write it

def file_with_name(filename)
  files = Dir["/GitHub/videos/#{filename}.{mkv,mp4,avi}"]
  raise "No file named #{filename}" if files.empty?
  raise "Several files: #{files}" if files.size > 1

  files.first
end

Instead of looping through all files in the folder, we try to find a file that starts with the right name, and that has an extension amongst mkv, mp4 and avi.

Then if no file is found, we raise an error. If several files share the same prefix, we raise an error.

In the case we found exactly one file, we return its name.

If you find good names for your variables and functions, and you try to break your code in smaller pieces (functions), you will be able to look at it in 2 months and understand it clearly.

You could even go further and use some classes to have be even more explicit with the season and the episode numbers: for now, we compute them, turn them into a string, and extract them from a string later on when displaying it. But I think it would be a bit out of scope here. Tell me if you want me to go further this path.

Well, that is it. Asking questions here is a good way to improve your coding skills, the rest is just practice!

\$\endgroup\$
  • \$\begingroup\$ Wow, thanks a tonne for the advice! This is really constructive and helpful. I will definitely take your advice on board for future projects. \$\endgroup\$ – Ryan Barker Oct 18 '18 at 7:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.