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;
}
$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!
ruby file-system iteration
$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.
add a comment |
$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!
ruby file-system iteration
$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
add a comment |
$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!
ruby file-system iteration
$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
ruby file-system iteration
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
add a comment |
$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
add a comment |
1 Answer
1
active
oldest
votes
$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 0
s 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
add a comment |
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
});
}
});
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
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
$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 0
s 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
add a comment |
$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 0
s 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
add a comment |
$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 0
s 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$
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 0
s 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!
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
add a comment |
$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
add a comment |
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.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
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
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
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
$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