Short Ruby program that opens a random video file from a directory Announcing the arrival of Valued Associate #679: Cesar Manara Planned maintenance scheduled April 17/18, 2019 at 00:00UTC (8:00pm US/Eastern)File Renaming with RubyImprove my script to organize a video collectionShorten code that gathers files from directoriesRemoving list of words from a text file in RubySimple Ruby directory navigator functionsOO coding style in short Ruby scriptsRuby command-line app that outputs from JSON fileCommand line tool, listing video files on a local drivePick a random file from a directory treeScheduled file sorting with Ruby

Writing Thesis: Copying from published papers

When communicating altitude with a '9' in it, should it be pronounced "nine hundred" or "niner hundred"?

How to rotate it perfectly?

When is phishing education going too far?

Am I ethically obligated to go into work on an off day if the reason is sudden?

Determine whether f is a function, an injection, a surjection

Stop battery usage [Ubuntu 18]

Geometric mean and geometric standard deviation

Interesting examples of non-locally compact topological groups

Autumning in love

If I can make up priors, why can't I make up posteriors?

Slither Like a Snake

Limit for e and 1/e

Estimated State payment too big --> money back; + 2018 Tax Reform

Two different pronunciation of "понял"

What LEGO pieces have "real-world" functionality?

What items from the Roman-age tech-level could be used to deter all creatures from entering a small area?

Can the prologue be the backstory of your main character?

Can't figure this one out.. What is the missing box?

I'm thinking of a number

How do you clear the ApexPages.getMessages() collection in a test?

Can I throw a sword that doesn't have the Thrown property at someone?

Did the new image of black hole confirm the general theory of relativity?

Need a suitable toxic chemical for a murder plot in my novel



Short Ruby program that opens a random video file from a directory



Announcing the arrival of Valued Associate #679: Cesar Manara
Planned maintenance scheduled April 17/18, 2019 at 00:00UTC (8:00pm US/Eastern)File Renaming with RubyImprove my script to organize a video collectionShorten code that gathers files from directoriesRemoving list of words from a text file in RubySimple Ruby directory navigator functionsOO coding style in short Ruby scriptsRuby command-line app that outputs from JSON fileCommand line tool, listing video files on a local drivePick a random file from a directory treeScheduled file sorting with Ruby



.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty margin-bottom:0;








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_num0#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!










share|improve this question











$endgroup$




bumped to the homepage by Community 14 mins ago


This question has answers that may be good or bad; the system has marked it active so that they can be reviewed.














  • $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

















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_num0#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!










share|improve this question











$endgroup$




bumped to the homepage by Community 14 mins ago


This question has answers that may be good or bad; the system has marked it active so that they can be reviewed.














  • $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













1












1








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_num0#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!










share|improve this question











$endgroup$




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_num0#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!







ruby file-system iteration






share|improve this question















share|improve this question













share|improve this question




share|improve this question








edited Sep 24 '18 at 14:03







Ryan Barker

















asked Sep 24 '18 at 12:06









Ryan BarkerRyan Barker

62




62





bumped to the homepage by Community 14 mins ago


This question has answers that may be good or bad; the system has marked it active so that they can be reviewed.







bumped to the homepage by Community 14 mins ago


This question has answers that may be good or bad; the system has marked it active so that they can be reviewed.













  • $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
















  • $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















$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$
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




$begingroup$
@FreezePhoenix thanks for the response, I have amended the title in accordance.
$endgroup$
– Ryan Barker
Sep 24 '18 at 12:54










1 Answer
1






active

oldest

votes


















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!






share|improve this answer









$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






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
);



);













draft saved

draft discarded


















StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f204273%2fshort-ruby-program-that-opens-a-random-video-file-from-a-directory%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









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!






share|improve this answer









$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















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!






share|improve this answer









$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













0












0








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!






share|improve this answer









$endgroup$



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!







share|improve this answer












share|improve this answer



share|improve this answer










answered Oct 16 '18 at 21:28









GuillaumeGuillaume

1064




1064











  • $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
















  • $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















$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




$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

















draft saved

draft discarded
















































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.




draft saved


draft discarded














StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f204273%2fshort-ruby-program-that-opens-a-random-video-file-from-a-directory%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

名間水力發電廠 目录 沿革 設施 鄰近設施 註釋 外部連結 导航菜单23°50′10″N 120°42′41″E / 23.83611°N 120.71139°E / 23.83611; 120.7113923°50′10″N 120°42′41″E / 23.83611°N 120.71139°E / 23.83611; 120.71139計畫概要原始内容臺灣第一座BOT 模式開發的水力發電廠-名間水力電廠名間水力發電廠 水利署首件BOT案原始内容《小檔案》名間電廠 首座BOT水力發電廠原始内容名間電廠BOT - 經濟部水利署中區水資源局

Prove that NP is closed under karp reduction?Space(n) not closed under Karp reductions - what about NTime(n)?Class P is closed under rotation?Prove or disprove that $NL$ is closed under polynomial many-one reductions$mathbfNC_2$ is closed under log-space reductionOn Karp reductionwhen can I know if a class (complexity) is closed under reduction (cook/karp)Check if class $PSPACE$ is closed under polyonomially space reductionIs NPSPACE also closed under polynomial-time reduction and under log-space reduction?Prove PSPACE is closed under complement?Prove PSPACE is closed under union?

Is my guitar’s action too high? Announcing the arrival of Valued Associate #679: Cesar Manara Planned maintenance scheduled April 23, 2019 at 23:30 UTC (7:30pm US/Eastern)Strings too stiff on a recently purchased acoustic guitar | Cort AD880CEIs the action of my guitar really high?Μy little finger is too weak to play guitarWith guitar, how long should I give my fingers to strengthen / callous?When playing a fret the guitar sounds mutedPlaying (Barre) chords up the guitar neckI think my guitar strings are wound too tight and I can't play barre chordsF barre chord on an SG guitarHow to find to the right strings of a barre chord by feel?High action on higher fret on my steel acoustic guitar