Code issues in jsonLoad/jsonLoads

[HolQue]

Hi Son,

in constructor of CJsonPreprocessor you initialize parameters with '':

self.jsonPath        = ''
self.masterFile      = ''

I would prefer to use 'None' as initial value. Because: An empty string can also be a valid value (not in this context but in general). Therefore using 'None' is more obvious.

In 'jsonLoad' you define the parameter 'masterFile':

def jsonLoad(self, jFile : str, masterFile : bool = True):

I am not happy with that. Because 'jsonLoad' is a public method for users of the JsonPreprocessor. For what kind of purpose shall a user need this parameter? What is the use case behind this?

'masterFile' seems to be an internal control flag. But in my opinion it is unfavorable to make such flags public.

I am not experienced with your code and therefore I am not sure if the following is a proper solution, but could you please check, if something like a 'masterFile auto detection' would be possible?

Not:

def jsonLoad(self, jFile : str, masterFile : bool = True):
   jFile = CString.NormalizePath(jFile, .....
   if masterFile:
       self.masterFile = jFile

But something like this instead:

def jsonLoad(self, jFile : str):

   jFile = CString.NormalizePath(jFile .....
   if self.masterFile is None:
       self.masterFile = jFile

The same with parameter 'firstLevel' in 'jsonLoads':

def jsonLoads(self, sJsonpContent : str, referenceDir : str = '', firstLevel : bool = True):

Also 'jsonLoads' is a public method for users of the JsonPreprocessor. For what kind of purpose shall a user need this parameter? What is the use case behind this?

[namsonx]

Hello Holger,

I agree with you that an internal control flag presenting in input parameters of a public method is not nice. I will try to find other solution in this case.

Regarding to the purpose of masterFile and firstLevel, because JP allows importing JSONP files recursively, so these parameters use to detect a root of JSONP file in case of jsonLoad method and an entry level of JSONP string in case of jsonLoads.

Thank you, Son

[HolQue]

Hi Son,

ok, you explained masterFile and firstLevel. But the meaning of these parameters and the relevance for users is still unclear to me. We need to give guidance in the documentation about that. I am still not convinced that users need to set masterFile and firstLevel in their codes.

I switched between

dictValues = oJsonPreprocessor.jsonLoads(sJSON, firstLevel=True)

and

dictValues = oJsonPreprocessor.jsonLoads(sJSON, firstLevel=False)

But I cannot see any difference in the results. Even if I set an invalid value

dictValues = oJsonPreprocessor.jsonLoads(sJSON, firstLevel="ABC")

I get the same results.

Seems that 'firstLevel' has no impact on the computation.

For me this makes no sense. Please rework your code.

[namsonx]

Hello Holger,

As I mentioned in my previous comment: I will update the code to remove internal control flag out of input parameters of a public method as your suggestion.

The firstLevel value will impact if sJSON has imported files.

Thank you, Son