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

Change PaginatedList.getList to return a generic typed List<E> instead of a List<Object> #105

Closed
nzall opened this issue Jan 21, 2020 · 12 comments

Comments

@nzall
Copy link

nzall commented Jan 21, 2020

It appears like in version 2.0 onwards, PaginatedList.getList() was changed to use List'''''' rather than a raw List without type safety. This is problematic because if someone extends this PaginatedList interface with their own generic wrapper around your raw getList, as we do, their code won't compile and instead throw warnings and errors about generic types and casting. Instead, if at all possible, PaginatedList should return a generic List, allowing users with a generic wrapper around your PaginatedList to drop the use of this wrapper entirely.

Is there any particular reason why PaginatedList.getList() returns a List<Object> rather than a List<E>?

@nzall nzall changed the title Please change PaginatedList.getList to return a generic typed List<E> instead of a List<Object> change PaginatedList.getList to return a generic typed List<E> instead of a List<Object> Jan 21, 2020
@nzall nzall changed the title change PaginatedList.getList to return a generic typed List<E> instead of a List<Object> Change PaginatedList.getList to return a generic typed List<E> instead of a List<Object> Jan 21, 2020
@hazendaz
Copy link
Owner

hazendaz commented Feb 4, 2020 via email

@void-spark
Copy link

Yup, ran into this as well

@hazendaz
Copy link
Owner

hazendaz commented Jul 9, 2021

Please submit a pull request for any change here.

@hazendaz
Copy link
Owner

Original code actually was Object throughout and had warnings where it was not set, I'm looking at making that specific one use <E> and seeing how that impacts other portions of code specifically where internally will just treate all as List as it originally was but should allow you then to do what you want.

@hazendaz
Copy link
Owner

To add to this, 2.x line was continued from 1.2. The original author had commits added 13 years ago called 'generics and cleanups' which changed all undefined to object internally (user fgiust).

@hazendaz
Copy link
Owner

Rather than using generics, used Row instead of Object as that is the physical object. A test case simply extends Row and that works as well so I suspect this issue is resolved as long as caller extends Row object, therefore I'm closing this as fixed on master. If any issue exists after next release, we can revisit.

@nzall
Copy link
Author

nzall commented Nov 28, 2021

I'm sorry, but we are simply unable to extend the Row object in our current setup. It's also not necessary at all right now to extend the Row object in the current implementation of the library.

@glively
Copy link

glively commented Nov 10, 2022

imo, it should not be List. That completely breaks the API for all existing users, and face it, 99.999% of the users of this tag are existing users looking to get rid of old deps. I have dozens and dozens of displayTag instances; there's no way I'm spending weeks re-plumbing my code to ... extend Row?. I used to be able to just set my results to the result of a JPA query and set my size. Now I cannot set my query results due to setList<List>?! I don't get it

@glively
Copy link

glively commented Nov 10, 2022

I should have formatted for HTML. I mean it should not return a list of Row

@hazendaz
Copy link
Owner

To add to my original comments, the original owner changed to List in 2014 per this commit

The actual code is a Row not Object. This was fixed to properly reflect that in use case stated although user opted to not address it.

Addressing that is not that complex. If you are reading data back from a database and loading it, not understanding why you simply cannot type it nor use IDE to search/replace. Its still a row. Its a paginated list after all and that is what is being displayed (rows).

@glively
Copy link

glively commented Nov 12, 2022

First of all, thank you for your work on this project, it is much appeciated. I don't understand where I need to extend Row. I work off 'domain objects' produced by JPA queries. We have a 'custom component' that implements PaginatedList we've use for a decade or more and it's worked great. Basically the component looks like this:

public class DisplayTagOptions<T> implements PaginatedList {
	...
	
	@Override
	public List<T> getList() {
		return list;
	}

	public void setList(List<T> list) {
		this.list = list;
	}
	...
}

And a Controller that would use the above component, looks like this:

@GetMapping(value="/list")
public String list(...) {
	...
	DisplayTagOptions<MyObject> displayTagOptions = new DisplayTagOptions<>(...);

	displayTagOptions.setList( myService().getMyObjects(...) );
	displayTagOptions.setFullListSize(  myService().countAllMyObjects(...) );

	model.addAttribute("displayTagOptions", displayTagOptions);
	...

	return getPage("list");
}

And on a JSP, it would be rendered like this:

<display:table name="displayTagOptions" htmlId="myObjectList" uid="myObjects" id="myObject" ... >
	<display:column style="width:10%" title="ID">
		<c:out value="${myObject.id}"/>
	</display:column>
        ...
</display:table>

Any feedback you have is greatly appreciated. Thank you

@hazendaz
Copy link
Owner

hazendaz commented Nov 12, 2022 via email

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

No branches or pull requests

4 participants