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

Append/Prepend overloads that take a lambda #69

Open
becdetat opened this issue Nov 13, 2013 · 6 comments
Open

Append/Prepend overloads that take a lambda #69

becdetat opened this issue Nov 13, 2013 · 6 comments

Comments

@becdetat
Copy link
Contributor

So I could do:

@s.FieldFor(x => x.Email).Append(() => {
    if (Model.Email == "") return "";
    return "<a href='mailto'/.......
})

without having to make a helper every time or do cray-cray inline logic.

@MattDavies
Copy link
Member

I think it might be more useful if the action gave you the property in question? ie:

@s.FieldFor(x => x.Email).Append(e => {
if (e == "") return "";
return "<a href='mailto'/.......
})

but otherwise I would probably just use a ternary in this situation, given the extra overload doesn't really make it any cleaner than this:

@s.FieldFor(x => x.Email).Append(
Model.Email == "" ? "" : "<a href='mailto'/.......";
)

Thoughts?

@becdetat
Copy link
Contributor Author

Giving the property is a good idea. My idea was that having the overload would allow arbitrary code, not just ternaries. Since this is the only way to inject markup for a field I would like the extra control.

@MattDavies
Copy link
Member

Sure - with the property this would be really cool then. @robdmoore might have other thoughts

@robdmoore
Copy link
Member

Lambdas and views don't play nicely together. I'd say @Helper or
complex code in view model is nicer tbh.

Unless there is a second overload that takes a Boolean property for
whether or not to display it.

@becdetat
Copy link
Contributor Author

Why don't views like lambdas? They're cute and cuddly. Everybody likes lambdas.

I was sort of inspired by jQuery's append() which takes an anonymous function. It's kinda just sugar but I like my sugar like I like my lambdas. In my views. I should stop.

I would agree with extracting it out into the viewmodel, not so much into a helper for a single case as it moves the declaration of a field (@s.FieldFor()) away from its implementation (the helper). But I would like to start out with a lambda and extract that out if the logic warrants it, example would be once it becomes complex enough to warrant unit testing. This syntax feels like a natural middle ground.

I don't think the boolean property would be very discoverable. maybe something like

@s.FieldFor(x => x.Foo).Append(x => ,,,).If(x => x.Foo != null)

@robdmoore
Copy link
Member

Basically two-fold: sometimes I've noticed multi-line constructs in views become really nasty or difficult (admittedly, I'm more thinking about fluent insterfaces; lambdas might be OK.

Also, the formatting and indentation of ReSharper and VS in razor views still leaves a lot to be desired.

So possibly it's not such a bad idea.

Re: moving out IHtmlString definitions to helpers I actually like that it moves it away from the field definition, but for the case you describe above I can see the value in keeping it with the field definition since it's small enough that you loose context in moving it away.

The .If thing reads cool, but isn't possible to implement and having a second level of fluent interface is confusing regardless (plus it will make @graemefoster cry :().

Let's run with an append that takes the Model itself as a parameter? I think that's more useful than the property itself and more consistent with all the other lambdas.

i.e.:

@s.FieldFor(x => x.Email).Append(m => {
    if (string.IsNullOrEmpty(m.Email))
        return new HtmlString("");
    return new HtmlString(string.Format("<a href=\"mailto:{0}\">{0}</a>", m.Email.ToHtml()));
})

Although I'm still leaning towards a helper or view model abstraction being cleaner given you have to wrap in the HtmlString (which both of your examples were missing).

@robdmoore robdmoore changed the title Append/Prepend overloads that take an action Append/Prepend overloads that take a lambda Apr 27, 2020
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

3 participants