Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem when adding Parse, Add and Loadrecordsets #90

Open
dtrillo opened this issue Mar 9, 2020 · 3 comments
Open

Problem when adding Parse, Add and Loadrecordsets #90

dtrillo opened this issue Mar 9, 2020 · 3 comments
Labels
enhancement fix-in-development A fix for this issue was implemented on current development branch.

Comments

@dtrillo
Copy link

dtrillo commented Mar 9, 2020

Hi.
I use the library adding data using Parse function, but also with the Add and LoadRecordset. the property name doesn't work as expected. Here is hoy I solved it!

I have notice some problems, that I have solved by defining my own LoadRecordset method.
`
' Load properties from an ADO RecordSet object into an array

' @param rs as ADODB.RecordSet

public sub LoadRecordSet(byref rs) ' Change 3.8.2

	LoadRecordSet2 rs, true

end sub

public sub LoadRecordSet3(byref rs) ' Change 3.8.2

	LoadRecordSet2 rs, false

end sub

public sub LoadRecordSet4(byref rs, prop_name) ' Change 3.8.2

	dim old_i

	old_i = i_defaultPropertyName

	i_defaultPropertyName = prop_name
	LoadRecordSet2 rs, true
	i_defaultPropertyName = old_i
end sub
' Load properties from an ADO RecordSet object into an array
' @param rs as ADODB.RecordSet
public sub LoadRecordSet2(byref rs, lcaseall) ' Change 3.8.2
	dim arr, obj, field
	set arr = new JSONArray
	while not rs.eof
		set obj = new JSONobject
		for each field in rs.fields
			k = field.name: if lcaseall then k = lcase(k)
			obj.Add k, field.value
		next
		arr.Push obj
		rs.movenext
	wend
	set obj = nothing
	add i_defaultPropertyName, arr
end sub`

With this 4 functions, I can get the desire outcome.
first of all, because I can assing a name to the recordset, not loosing the previous name.

I notice you have not add the feature of lowercase properties (here included).

@rcdmk
Copy link
Owner

rcdmk commented Mar 9, 2020

Hi @dtrillo,

That is cool, thanks!

You can create a PR to add that functionality.
Just need to fork the repo, create the change and send the PR. It will get merged with your contribution to the repo.

Also, what about giving more clear names to the methods there?
My suggestions would be:

  • rename LoadRecordSet2 to LoadRecordSetWithCase
  • remove the other ones and change places that call them to call the LoadRecordSetWithCase using the proper flags, because that seems to be more clear and we don't introduce methods to wrap a boolean.

What do you think?

@dtrillo
Copy link
Author

dtrillo commented Mar 10, 2020

I don't use GitHub very ofter because I don't have it available except at home, and this is a feature of my work. I will try to make the PR in the next days.

@rcdmk rcdmk added enhancement fix-in-development A fix for this issue was implemented on current development branch. labels Sep 30, 2022
@rcdmk
Copy link
Owner

rcdmk commented Oct 4, 2022

I have a possible fix for this in development (4b5fef0), if you still wanna try that out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement fix-in-development A fix for this issue was implemented on current development branch.
Projects
None yet
Development

No branches or pull requests

2 participants