search

Dyalog / Jarvis

APL-based web service framework supporting JSON or REST
https://dyalog.github.io/Jarvis/
MIT License
33 stars 8 forks source link

Is it necessary to expunge the connection here? #5

Closed arcfide closed 4 years ago

arcfide commented 4 years ago

It appears to me that expunging the obj is handled in the call to HandleRequest when it returns a 1. Is there a reason that we use an r=0 and then manually expunge the object at the following line?

https://github.com/Dyalog/Jarvis/blob/ccd05ffa357f880252ae630080c63efd3b1fb8d7/Source/Jarvis.dyalog#L539

arcfide commented 4 years ago

As mentioned in the pull request, if I understand this correctly, I think it would be better to expunge on the outside, where the object is created.

arcfide commented 4 years ago

I notice now that making a response includes expunging the object as a part of its work. This makes me wonder now whether it is even necessary to expunge the object when we return from HandleRequest?

arcfide commented 4 years ago

I'm referring to this apparently excessive expunge: https://github.com/Dyalog/Jarvis/blob/ccd05ffa357f880252ae630080c63efd3b1fb8d7/Source/Jarvis.dyalog#L446

arcfide commented 4 years ago

d2c5e53f8ca17c9764ba60246af8913d2bbe327c removes the extra expunge here, and now the only expunges are on the outside caller to HandleRequest and inside of the Respond function. Are they both necessary?

bpbecker commented 4 years ago

Yeah, the duplicate expunge was a remnant of cope that was lifted from elsewhere. While it's harmless I'll address it.