vankasteelj / opensubtitles-api

nodejs opensubtitles.org api wrapper for downloading and uploading subtitles in multiple langs
110 stars 33 forks source link

Update search.js #24

Closed codebykenny closed 7 years ago

codebykenny commented 7 years ago

This fixes #23

vankasteelj commented 7 years ago

I don't see how this could ever make a difference. Undefined, false, 0 or null is the same in this case

codebykenny commented 7 years ago

So you have fileTagsDic which is an object of all tags found in the file name set to false.

The goal of the loop is to iterate through all of the tags found in sub name and set the false values to true and count the matches.

Perfect.

However the way you're checking if that value is set to false is the following

 if (!fileTagsDic[subTag]) {
     matches++;
}

This is flawed because you are increasing matches not only of values set to false, but also values that don't exist.

!fileTagsDic[subTag] is always going to be true for every value that is not in the object.

eg.

filename: The.Big.Bang
subname1: Really.Long.Sub.Name.With.Many.Tags
subname2: The.Big.Bang.720p

fileTagsDic = {
    'the': false,
    'big': false,
    'bang': false
}

//loop..
if (!fileTagsDic[subTag]) {
    fileTagsDic[subTag] = true
    matches++
}

//Results subname1//
!fileTagsDic['Really'] // true, matches ++
!fileTagsDic['Long'] // true, matches ++
!fileTagsDic['Sub'] // true, matches ++
!fileTagsDic['Name'] // true, matches ++
!fileTagsDic['With'] // true, matches ++
!fileTagsDic['Many'] // true, matches ++
!fileTagsDic['Tags'] // true, matches ++

matches = 7 points despite not having any matching tags

//Results subname2//
!fileTagsDic['The'] // true, matches ++
!fileTagsDic['Big'] // true, matches ++
!fileTagsDic['Bang'] // true, matches ++
!fileTagsDic['720p'] // true, matches ++

matches = 4 points

----

subname1: Really.Long.Sub.Name.With.Many.Tags (7 points)
subname2: The.Big.Bang.720p (4 points)

Essentially matching tags is broken at the moment

Hope this helps understand!

vankasteelj commented 7 years ago

ok, I think I get it.