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

Implement DataList as iterator #15

Open
mkly opened this issue Mar 11, 2014 · 7 comments
Open

Implement DataList as iterator #15

mkly opened this issue Mar 11, 2014 · 7 comments

Comments

@mkly
Copy link
Owner

mkly commented Mar 11, 2014

See Remo/concrete5@f371ab2

@Remo
Copy link
Contributor

Remo commented Mar 11, 2014

I wanted to implement this but realized that it requires a change in DatabaseItemList as well. The current implementation has a GetAll which I think should be avoided. We can implement an iterator with it but it feels strange to combine it with an iterator.. Any thoughts on this? Wanted to get that iterator stuff into 5.7 but haven't had enough reasons to spend time on it. Customer projects come first..

@mkly
Copy link
Owner Author

mkly commented Mar 11, 2014

So very true about customer projects. That said, could we not just implement our own get() method and add the iterator methods to the DataList class? I'll leave this to investigate later.

@Remo
Copy link
Contributor

Remo commented Mar 11, 2014

Sure, I'm just not a big fan of overriding such methods.. I'll try to implement this tomorrow. From: Mike LaySent: Tuesday, March 11, 2014 22:30To: mkly/DataReply To: mkly/DataCc: Remo LaubacherSubject: Re: [Data] Implement DataList as iterator (#15)So very true about customer projects. That said, could we not just implement our own get() method and add the iterator methods to the DataList class? I'll leave this to investigate later.

—Reply to this email directly or view it on GitHub.

@mkly
Copy link
Owner Author

mkly commented Mar 11, 2014

Awesome. No rush though. Data still needs some work. For example, I'm not sure if all the child attributes of the relation attributes are being deleted properly, and I need to write a whole bunch more tests!!! Thanks again for the input.

@Remo
Copy link
Contributor

Remo commented Mar 12, 2014

Well, I'm somehow in a rush because we need that export functionality and I want that to be on top of an iterator to keep the memory consumption at a reasonable level. The PR linked above is still a work in progress, the first element is empty and I'll have to add some unit tests as well. I just created the PR to make my progress "visible"..

@mkly
Copy link
Owner Author

mkly commented Mar 12, 2014

That's great to hear that you are getting some use out of this in production. I wouldn't stress out too much about unit tests right now. I haven't posted instructions on how to even get the test harness up. I can always write those underneath any additions you might make. Is it the csv export you are talking about for the data itself?

There is actually a bit of a start to import "Datas" right now in an xml format. I decided to wait on it because of time contraints.

https://github.com/mkly/Data/blob/master/src/data/models/data.php#L155-L171

That is for Datas nested in DataTypes. Its a work in progress and unused functionality, so you can change that method if you have a preference to it being different.

@Remo
Copy link
Contributor

Remo commented Mar 13, 2014

Yes, we need all the attributes specified for a data type in columns with a row for each entry in the data table.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants