neda1985 / search

0 stars 1 forks source link

review queryBuilder func #6

Open neda1985 opened 4 years ago

neda1985 commented 4 years ago

@fzerorubigd can you please review this func it looks a bit crazy :( actually elastic query should look like this : {"query": { "bool" : { "must" : [ {"term" : { "genres.name" :{"value ":"genre" } }, {"term" : { "moods.name": {"value ":"moods" } }, {"term" : { "has_vocal" : {"value ":"hasVocal" } } , ] } } } }

fzerorubigd commented 4 years ago

it's not that bad, just a few notes :

https://github.com/neda1985/search/blob/d7b1902c74c71fd81b43c333b27adefdbc7e1148/main.go#L136

Generally, its better to not create an empty array with make, jsut go for var must []match but in this situation I suggest doing it this way: must := make([]match, 0, 3) adding capacity when you know the size beforehand is the best practice.

Another approach is instead of using map, use struct. Define your structure based on the JSON you want (this tool can help https://mholt.github.io/json-to-go/ ) and it is much faster and memory-efficient that way.

neda1985 commented 4 years ago

I remove must := make([]match, 0)

neda1985 commented 4 years ago

will refactor again and get back to you thanks :)

fzerorubigd commented 4 years ago

I remove must := make([]match, 0)

Since you know at max you are going to add 3 elements to this slice, I suggest this change : must := make([]math, 0, 3)

neda1985 commented 4 years ago

ok yeah make way more sense

neda1985 commented 4 years ago

correct me if I am wrong must := make([]match, 0) would allocate unnecessary memory right ? but when we define size just the defined one will be allocated

fzerorubigd commented 4 years ago

Actually No. var x []string defines a variable x, with nil as its value, next time you want to append value to it, it allocates the array (under any slice there is an array in Go)

x := make([]string,0) define a variable x but the value is an empty slice, it allocate an empty array (it's empty, but it occupies memory, next time when you want to append a value to it, it has to allocate once more, so the first allocation is kind of pointless)

x := make([]string,0,3) it allocates at the beginning, but it is empty. the capacity is 3, so for the first 3 appends, there are no allocations. this is the best when you know the size of slice at first.

neda1985 commented 4 years ago

ahhhhhhhh , thanks for explanation , got it