You are reading content from Scuttlebutt
@dangerousbeans

How should I make this DRY-err

 if(name.endsWith('.mp4'))
 {
   content_file = file
 }
 else if(name.endsWith('.m4v'))
 {
   content_file = file
 }
 else if(name.endsWith('.webm'))
 {
   content_file = file
 }
 else if(name.endsWith('.ogg'))
 {
   content_file = file
 }
 else if(name.endsWith('.mp3'))
 {
   content_file = file
 }
@nico_forgotten_and_lost

A regular expression matching possible filename endings should do it

@nichoth
var names = ['.ogg', '.webm', '.mp4', /* etc */ ]
var l = names.length
while (l--) {
    if name.endsWith(names[l]) {
        content_file = file
        break
    }
}
@nico_forgotten_and_lost

^.*.(mp4|m4v|webm|ogg|mp3)$

@nico_forgotten_and_lost

var exp = new RegExp("^.*.(mp4|m4v|webm|ogg|mp3)$);
var matches = exp.test(name);
if (matches) {
content_file = file;
}

@joeyh
if any name.endsWith [".mp4", ".m4v", ".webm", ".ogg", ".mp3"]
  ...

Pardon the haskell conversion. If your language does not have an any function, it's worth implementing one. You probably already have a map, and you also need an or that checks if any value in a list is True.

any f l = or (map f l)
@Dominic

@nicolasini
you should escape the separator, and you don't actually need to match the start if you match the end, and probably also make it case insensitive (i). /\.(mp4|m4v|webm|ogg|mp3)$/i.test(name)

@mikey

lately i've been using ramda:

const { any, endsWith } = require('ramda')

const isMediaFileName = any(endsWith([".mp4", ".m4v", ".webm", ".ogg", ".mp3"]))

if (isMediaFileName(fileName)) {
  // ...
}

also easy to implement without ramda:

const endsWith = (ending) => (string) => string.endsWith(ending)
const any = (array) => (predicate) => array.some(predicate)
@mix

I'd go

var ACCEPTED_FORMATS = ['mp4, 'm4v', 'webm', 'ogg', 'mp3']
var extension = name.match(/.*\.(\w+)$/)[1]  // or use some module

if (~ACCEPTED_FORMATS.indexOf(extension)) {
  content_file = file
}

that ~ is a gross js hack... and lodash can make a bunch of this pretty obsolete if you want to use that

@mix

@mikey that's sweet.

I can't quite see how you would call your self-rolled closure together to get what that ramda does. i.e. I don't thinkg any(endsWith(array)) would work for what you wrote would it?

@neftaly

@dinosaur I think it's nooot quite right. Fortunately, Ramda is super fun!

const hasExt = v => R.any(
  R.flip(R.endsWith)(v),
  [".mp4", ".m4v", ".webm", ".ogg", ".mp3"]
);

repl

@neftaly

Also if you like pointfree:

const hasExt = R.compose(
  R.any(R.__, [".mp4", ".m4v", ".webm", ".ogg", ".mp3"]),
  R.flip(R.endsWith)
);
@nanomonkey

Perhaps a switch statement

switch (name.split(".").pop()) {
    case 'txt':
       // do something different
    case 'm4v':
    case 'ogg':
    case 'mp3':
    case 'webm':
    case 'mp4':
        //this will catch all of the above cases after 'txt'
        content_file = file
        break;
    default:
        content_file = file
}
@mikey

ahh @mixxx and @neftaly y'all are right, i got ahead of myself. :sweat_smile:

@alanshaw
if (['mp4, 'm4v', 'webm', 'ogg', 'mp3'].some(ext => name.endsWith(ext))) {
  content_file = file
}
@2mas

Ramda seams awesome. There's a lot of documentation. Good to see nice digestible bite sized solutions!

@mix

good to know about #endsWith, haven't seen it before.

Also, this is a fun game, thanks @dangerousbeans !!!

@mix

anyone notice this is stack-overflowy ???
we could make patch-overflow the goto resource for all things buttz.

dogroll !, i mean dogfood :dog: :hamburger:

@agentofuser
Voted this
@joeyh

Interesting to look over this set of alternatives with a view to the total complexity of the code, not merely repetitiveness.

The loop has a potential fence post error or two. One regexp had an escaping bug. Quite a lot of the solutions need a variable, which makes name conflicts a concern, or adds complexity to encapsulate those. There's strange punctuation javascript hacks that junior JS developers (such as myself) won't understand. There's a tricky flipping of endsWith to get the arguments in the right order in a point-free context.

From a complexity point of view, the switch statement seems like the best alternative. Except it has a bug in the default case, which I think should do nothing, and of course it's easy to forget a break in a switch and shoot a current or future foot.

Also serveral alternatives weren't able to use the original endsWith and had to implement one that's more suitable. Which I suspect is a subtle part of the original problem, that method style of function reduces flexability.

Perhaps the least complex alternative is the original nested if..

@amnovak

We're in JS, right? I think the clearest way is probably:

for (ext of ['.mp4', '.m4v', '.webm', '.ogg', '.mp3']) {
    if (name.endsWith(ext)) {
        content_file = name
        break
    }
}

Make the list of supported extensions a constant if you feel like it. Also make sure that name is already lower-case, or add in a toLowerCase().

You also might want to just parse the extension from the filename, and do a key lookup in an object of supported ones, if you have lots of these. It might be faster than a linear scan.

@amnovak

Erm, content_file = file, rather.

Make sure you write a unit test for this because apparently it's super hard.

@Dominic

another approach:

var accepted = {
  mp3: true, mp4: true, m4v: true, webm:true, ogg:true
}
var ext = name.substring(name.lastIndexOf('.')+1)

if(accepted[ext])
  content_file = file

refactor for readability...

if(({
  mp3:1, mp4:1, m4v:1, webm:1, ogg:1
})[name.substring(name.lastIndexOf('.')+1)])
  content_file = file
User has chosen not to be hosted publicly
@neftaly

I would do it the @alanshaw
way TBH (if it wasn't inside a R.compose)

if (['.mp4, '.m4v', '.webm', '.ogg', '.mp3'].some(::name.endsWith)) {
  content_file = file
}
Join Scuttlebutt now