2017-06-27 17:30 GMT+02:00 Fabien COELHO <coe...@cri.ensmp.fr>: > > Hello Pavel, > > We can introduce macro SetVariableBool(vars, varname, bool) instead >>> >>> SetVariable(pset.vars, "ERROR", "FALSE"); >>> >> >> I checked source code, and it requires little bit more harder refactoring >> because now we have SetVariableBool - what is unhappy name, because it >> initialize variable to ON value. It is question what is better name? >> > > The boolean values (on/off 1/0 true/false...) accepted for pg settings is > probably convenient but also somehow fuzzy. > > From a programming point of view, I like booleans to have either true or > false values, and nothing else. > > I agree that the existing "SetVariableBool" function is a misnommer, it > should be "SetVariableOn" given what it does, and it is not what we need. >
switching default setting from ON to TRUE requires wider discussion - in this moment I like to have special function "SetVariableON". > > Here is a v4 which attempts to extend & reuse the function. People might > be surprised that TRUE is used where ON was used before, so I'm not sure. > > I found more interesting issue - the code of SetResultVariables is >> partially redundant with AcceptResult - maybe the switch there can be >> shared. >> > > I agree that there is some common structure, but ISTM that the > AcceptResult function is called in a variety of situation where variables > are not to be set (eg "internal" queries, not user provided queries), so I > thought it best to keep the two apart. I understand, but It is not nice, really - maybe only switch can be moved to some inlining function like IsSuccess() - more .. with this function, the SetResultVariables function will be more cleaner Regards Pavel > > > -- > Fabien.
