WebTorrent播放器
WebTorrent播放器
提问于 2016-02-24 13:16:15
我已经使用HTML5和CSS3很长一段时间了，但是我一直避免使用JavaScript，因为我相信它是最经常使用的，同时也有写得不好的倾向。所以我坚持了下来，直到我需要学习它来使用一些非常酷和有用的东西。卖给我的项目是WebTorrent。
我确实有编程经验(我所知道的最相似的语言是PHP和Java)，所以我对标准有一些了解，但我不知道JavaScript的最佳实践。这段代码当然可以工作，以后还会进行改进(它更像是一个技术演示，而不是任何东西)，所以我更喜欢对语法和一般方法的批评，而不是函数，除非我做了一些根本错误的事情。
我没有进入这个盲点，尽管它很诱人。我的介绍是MDN的重新介绍JavaScript，我发现这很有帮助。所有这些代码都在页面的<head>中，所以如果我要把它移到其他地方或者在元素上调用它，请告诉我。
<script src="https://cdn.jsdelivr.net/webtorrent/latest/webtorrent.min.js"></script>
<script>
var client = new WebTorrent();
// Torrent ID
var torrentId = 'redacted WebTorrent magnet link - contains 2 audio files, 1 video file, 1 image';
client.add(torrentId, function ontorrent(torrent){
/*
Gameplan:
Load Content (done by this point)
Case empty torrent
Case no playable media
warn user
break
Case multiple playable media
ask which file(s) to play
Case one playable media
play media
break
*/
// Compatible Media Formats
var MEDIA_EXT = ['mp4', 'm4v', 'm4v', 'webm', 'm4a', 'mp3', 'wav', 'aac', 'ogg', 'oga'];
function getExt(name){
// Not own work: http://stackoverflow.com/questions/190852/how-can-i-get-file-extensions-with-javascript
return name.substr((~-name.lastIndexOf(".") >>> 0) + 2);
}
// Status logger
var logElement = document.getElementById('status');
function pStatus(msg){ logElement.innerHTML += "<br/>" + msg };
var numFiles = torrent.files.length;
// Check for empty torrent
if(numFiles == 0){
pStatus("No files found! Cannot render media.")
return;
}
// Find all playable media
var playable = new Array();
torrent.files.forEach(function(file){
pStatus(" - Found file: " + file.name);
if(MEDIA_EXT.indexOf(getExt(file.name.toLowerCase())) > 0){
playable.push(file);
}
});
playable.forEach(function(file){
pStatus("File " + file.name + " is usable.");
});
if(playable.length === 1){
playable[0].appendTo(document.getElementById('target'));
}else{
do{
var index = window.prompt("Multiple files found. Please choose the index you would like to play.");
}while(playable[index] == undefined);
var file = playable[index];
pStatus("Choosing index " + index + ": " + file.name + "...");
file.appendTo(document.getElementById('target'));
pStatus("Now loading " + file.name);
document.title = file.name;
}
});
</script>回答 3
Code Review用户
回答已采纳
发布于 2016-02-24 16:59:32
在回顾之后，我提出了以下几点意见：
- 你真的不应该做内联脚本，把它放在一个单独的.js文件中
- 没有无意义的注释，您应该删除
//Torrent ID - 变量应该在lowerCamelCase
MEDIA_EXT->mediaExtensions中，甚至在validFileExtensions中。 - 我喜欢这样的归因，虽然我倾向于把它放在
function之前，这样它就不会分散注意力。通常，只使用一次的一行函数是没有意义的，但是由于样式非常不同，所以我会保留它。 - 始终如一地编写函数，不要使pStatus成为一行
- 您只需要numFiles一次，要退出，我不会为此使用单独的var
- 我不喜欢游戏计划的评论，它并不真正属于那里，也许在设计文档中更好。
new Array();->在风格上更好地使用var playableMedia = [];- 我将研究[].filter，如果您要运行，您最好选择正确的函数
- 您已经知道了所有可播放的文件，如果您倾向于处理大量的文件，我将避免第二个循环。
- 你可以只做‘提示符’而不是‘窗口提示’，理想情况下你永远不要使用提示符;)
- 理想情况下，顶部有一个
var声明，而不是整个位置。 - 理想情况下，使用一种形式的引号，
'或"，我倾向于使用' - 处理1个文件与处理多个文件不一致，似乎是错误的。
- 使用JsHint.com
- 你少了分号
- 它会告诉你使用
if(numFiles === 0)，尽管我会使用if(!numFiles)
我会重构代码：
//No script tags, I assume this in a separate js now
var client = new WebTorrent(),
torrentId = 'redacted WebTorrent magnet link - contains 2 audio files, 1 video file, 1 image';
client.add(torrentId, function ontorrent(torrent) {
// Compatible Media Formats
var validFileExtensions = ["mp4", "m4v", "m4v", "webm", "m4a", "mp3", "wav", "aac", "ogg", "oga"],
playable = [],
usableLog = "",
index,
file;
// Not own work: http://stackoverflow.com/questions/190852/how-can-i-get-file-extensions-with-javascript
function getExt(name) {
return name.substr((~-name.lastIndexOf(".") >>> 0) + 2);
}
function updateStatus(msg) {
document.getElementById('status').innerHTML += "<br/>" + msg;
}
// Check for empty torrent
if (!torrent.files.length) {
updateStatus("No files found! Cannot render media.");
return;
}
// Find all playable media
playable = torrent.files.filter(function(file) {
updateStatus(" - Found file: " + file.name);
if (validFileExtensions.indexOf(getExt(file.name.toLowerCase())) > 0) {
usableLog += "<br/>File " + file.name + " is usable.";
return true;
}
return false;
});
updateStatus(usableLog || "No files were usable");
//What file should we play
if (playable.length === 1) {
index = 0;
} else {
do {
index = prompt("Multiple files found. Please choose the index you would like to play.");
} while (!playable[index]);
}
//Inform the user and play the file
file = playable[index];
updateStatus("Choosing index " + index + ": " + file.name + "...");
file.appendTo(document.getElementById('target'));
updateStatus("Now loading " + file.name);
document.title = file.name;
});Code Review用户
发布于 2016-02-24 13:30:22
- 用好的名字代替注释。与写入: //兼容媒体格式var MEDIA_EXT ='mp4'，'m4v'，'m4v'，'webm'，'m4a'，'mp3'，'wav'，'aac'，'ogg'，'oga‘不同，编写: var compatibleMediaExt = ['mp4‘.
- 在函数中封装更多的逻辑。不要写: if(MEDIA_EXT.indexOf(getExt(file.name.toLowerCase())) > 0){ playable.push(文件)；}只需将
getExt重命名为isValidFile并插入getExt的内容，与0相比，转换为更低的内容。 - 用变量描述
if语句，如果(playable.length === 1)不能说明该语句的含义。如果您编写: var oneFileFound = (playable.length === 1)和If (oneFileFound)，那么所有内容都将更具可读性。 - 为变量选择更好的名称。对于文件数组来说，
playable不是一个好的选择。files会好得多。
Code Review用户
发布于 2016-02-25 17:24:21
还没有人建议将代码变成闭包，这样就不会污染全局命名空间：
(() => {
var client = new WebTorrent();
...
})();页面原文内容由Code Review提供。腾讯云小微IT领域专用引擎提供翻译支持
原文链接：
https://codereview.stackexchange.com/questions/120966
复制相关文章
相似问题
腾讯云开发者
