Short Ruby program that opens a random video file from a directory Announcing the arrival of...

Why is "Captain Marvel" translated as male in Portugal?

Problem when applying foreach loop

Fishing simulator

Are my PIs rude or am I just being too sensitive?

If A makes B more likely then B makes A more likely"

Losing the Initialization Vector in Cipher Block Chaining

When is phishing education going too far?

Communication vs. Technical skills ,which is more relevant for today's QA engineer positions?

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

What's the difference between (size_t)-1 and ~0?

Stars Make Stars

I'm thinking of a number

Why is there no army of Iron-Mans in the MCU?

Why does this iterative way of solving of equation work?

Replacing HDD with SSD; what about non-APFS/APFS?

How are presidential pardons supposed to be used?

What computer would be fastest for Mathematica Home Edition?

Can a zero nonce be safely used with AES-GCM if the key is random and never used again?

What do you call a plan that's an alternative plan in case your initial plan fails?

Aligning matrix of nodes with grid

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

Passing functions in C++

What was the last x86 CPU that did not have the x87 floating-point unit built in?

Strange behaviour of Check



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_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!










share|improve this question











$endgroup$




bumped to the homepage by Community 9 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_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!










share|improve this question











$endgroup$




bumped to the homepage by Community 9 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_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!










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_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!







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

Fairchild Swearingen Metro Inhaltsverzeichnis Geschichte | Innenausstattung | Nutzung | Zwischenfälle...

Pilgersdorf Inhaltsverzeichnis Geografie | Geschichte | Bevölkerungsentwicklung | Politik | Kultur...

Marineschifffahrtleitung Inhaltsverzeichnis Geschichte | Heutige Organisation der NATO | Nationale und...