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

Switch: Clean up logic #513

Closed
melloware opened this issue Dec 6, 2017 · 4 comments
Closed

Switch: Clean up logic #513

melloware opened this issue Dec 6, 2017 · 4 comments
Assignees
Milestone

Comments

@melloware
Copy link
Member

melloware commented Dec 6, 2017

Reported on the forums: https://forum.primefaces.org/viewtopic.php?f=14&t=39806

public enum FooEnum
{
    A, B
}

public Bean
{
    private FooEnum = A;
}
<pe:switch value="#{bean.fooEnum}" >
	<pe:case value="A">
		A
	</pe:case>
	<pe:case value="B">
		B
	</pe:case>
</pe:switch>
@melloware melloware self-assigned this Dec 6, 2017
@melloware melloware added this to the 6.2.0 milestone Dec 6, 2017
@tandraschko
Copy link
Member

-1
pe:case value attr can be set to a enum via p:importEnum

@melloware
Copy link
Member Author

melloware commented Dec 6, 2017

it's a harmless change that tests for object equality and then if objects don't equal check for String representation equality. Plus it cleans up a mess of ==null != null logic.

Current code:

if ((caseComponent.getValue() == null && this.getValue() != null)
          || (this.getValue() == null && caseComponent.getValue() != null)) {
    continue;
}
if ((caseComponent.getValue() == null && this.getValue() == null)
            || caseComponent.getValue().equals(this.getValue())) {

     caseToRender = caseComponent;
     // mustn't break, because need to set rendered=false to other cases (e.g. for visitTree correctness)
}

New code (cleaner and catches this enum case):

Object evaluate = this.getValue();
Object caseValue = caseComponent.getValue();

if (ObjectUtils.equals(evaluate, caseValue) 
             || StringUtils.equalsIgnoreCase(String.valueOf(evaluate), String.valueOf(caseValue))) {
      caseToRender = caseComponent;
       // must not break, because need to set rendered=false to other cases (e.g. for visitTree correctness)
}

@tandraschko
Copy link
Member

I wouldnt do it personally. A string is not a enum, i introduced importEnum long time ago for this cases.
It would be just ok for me if JSF will do the same for action listeners, if you pass a string and the method param is a enum.

@melloware
Copy link
Member Author

OK I can live with that. I switched it to just this which does exactly the same as the original code with 1 line of code and not confusing "continue" statement etc.

final Object evaluate = this.getValue();
final Object caseValue = caseComponent.getValue();

 // TODO: switch this to Objects.equals in Java7
if (ObjectUtils.equals(evaluate, caseValue))) {
    caseToRender = caseComponent;
 }

@melloware melloware changed the title Switch: Support Enum Switch: Clean up logic Dec 6, 2017
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