On Fri, Oct 20, 2023 at 2:52 AM Alena Rybakina <lena.riback...@yandex.ru> wrote:
> Thank you for your work on the subject. > Thanks for taking an interest in this patch. > During review your patch I didn't understand why are you checking that the > variable is path and not new_path of type T_SamplePath (I highlighted it)? > > Is it a typo and should be new_path? > I don't think there is any difference: path and new_path are the same pointer in this case. > Besides, it may not be important, but reviewing your code all the time > stumbled on the statement of the comments while reading it (I highlighted > it too). This: > > * path_is_reparameterizable_by_child > * Given a path parameterized by the parent of the given child > relation, > * see if it can be translated to be parameterized by the child > relation. > * > * This must return true if *and only if *reparameterize_path_by_child() > * would succeed on this path. > > Maybe is it better to rewrite it simplier: > > * This must return true *only if *reparameterize_path_by_child() > * would succeed on this path. > I don't think so. "if and only if" is more accurate to me. > And can we add assert in reparameterize_pathlist_by_child function that > pathlist is not a NIL, because according to the comment it needs to be > added there: > Hmm, I'm not sure, as in REPARAMETERIZE_CHILD_PATH_LIST we have already explicitly checked that the pathlist is not NIL. Thanks Richard
