Hacker News new | past | comments | ask | show | jobs | submit login

I'm sure this is partly because I don't read F#, but it looks like they've moved all the complexity into meta-programming madness, this is just being way to clever to play code golf at a high level, this is exactly the sort of complexity we should be fighting against.

Even the initial c# version was over complicated. The complex fluent interface with lambdas and callbacks could be done with a few if statements that would be simpler, faster and require no knowledge of the FluentValidation library. Unnecessary getters and setters to satisfy the encapsulation gods.

If you want to fight complexity got back to basics, you can have a static method returning a validation result with code like this:

  if (!string.IsNullOrEmpty(card.CardNumber) && CardNumberRegex.IsMatch(card.CardNumber))
    validations.add("Oh my");
Converting if statements to more elaborate constructs is creating complexity not fighting it.



The problem is when you want validation errors which contains the field name, and a descriptive error message. Oh you want them localised as well?

You then want each field to be validated individually. So you get an error for each field which is wrong.

So you have if statements for each field creating a localised validation error object then placing in a list.

You have 8 fields coming in on your request. It's starting to look like a big method now with 8 if statements creating these localised validation objects.

You also want to share your validation rules between different use cases.

FluentValidation makes that quite quick and terse to achieve compared to simple if statements.


> The problem is when you want validation errors which contains the field name, and a descriptive error message. Oh you want them localised as well?

So the above example would become something like this:

  if (!string.IsNullOrEmpty(card.CardNumber) && CardNumberRegex.IsMatch(card.CardNumber))
    validationContext.add("CardNumber", Localizer.MessageFor("InvalidCCNumber"));
  if (x.ExpirationMonth < 1 || x.ExpirationMonth > 12)
    validationContext.add("ExpirationMonth", Localizer.MessageFor("InvalidCCExpiration"));
Throw in some lambda's for the property name and static strings for the message names if you really need to be type safe. Also I'm not sure if FluentValidator handles this, but you need somewhere for root level errors, not all errors map neatly to a property.

> You have 8 fields coming in on your request. It's starting to look like a big method now with 8 if statements creating these localised validation objects.

There's no local state, the errors are stored in a glorified dictionary, it's a simple imperative series of if statements that anyone who's gone beyond hello world in any language can understand. Big methods are not bad just because they're big (not that 8 if statements is big), they're bad when there is a lot of mutable state that the programmer has to track in their head, validation logic rarely has this problem. It would be fine if there were 1000 properties because the complexity is flat.

> You also want to share your validation rules between different use cases.

So you make a function. It doesn't look like FluentValidator offers any improvement here, it seems like custom rules with this library basically just wrap a function call: https://fluentvalidation.net/start#including-rules or you create a "function" at runtime with rulesets: https://fluentvalidation.net/start#including-rules

> FluentValidation makes that quite quick and terse to achieve compared to simple if statements.

From the examples I'm not sure it's any quicker or more terse after you include the extra boilerplate setup. All it seems to do is turn if statements into where/must calls, for loops into RuleForEach calls and functions into custom RuleSets. It also adds the complexity of using a library.


Your starting to build your own validation library inside that validation context. Inside I presume it must be creating some kind of error object to a list.

Next problem, rename a field on the object using a refactoring tool. You now have to change validation code to change the field name. You may forget about the validation code if your not looking at it. You have good tests though so you would probably would catch it. But you want it to be automatic. Maybe nameof?

The class is getting a lot responsibility, and you want to seperate validation out into its own class responsible for that. Maybe extract to a validation object which operates on a request/command class?

Might point is you eventually you end up building something like fluent validation. With own set of default rules, validation classes etc. Maybe fluent validation is overly complicated but I'd rather get the speed boost of using a well tested library that I already know instead of gradually refactoring into something custom.


> Your starting to build your own validation library inside that validation context.

I'm building a simple composite data structure, outside of NPM this is not remotely a library. With a bit of luck not even that, I'm a bit rusty but I think the MVC framework had one built in to handle this.

> Next problem

I already addressed it, a lambda function instead of a string that can extract the property name, but your right that nameof might be a better option these days.

> The class is getting a lot responsibility, and you want to seperate validation out into its own class responsible for that. Maybe extract to a validation object which operates on a request/command class?

I already have, the actual validation is in a static method somewhere, so it has only one responsibility and the validation context is mostly a simple data structure, that's not too much responsibility.

> Might point is you eventually you end up building something like fluent validation. With own set of default rules, validation classes etc. Maybe fluent validation is overly complicated but I'd rather get the speed boost of using a well tested library that I already know instead of gradually refactoring into something custom.

It's not an unbounded problem with lot's of gotchas down the line, it's a well known and simple to solve problem that practically everyone has seen before. What you're ignoring is the complexity of adding a dependency in general and the complexity of this library in particular. Adding a dependency is not a free lunch, it has a cost in time, mental overhead and maintenance. This particular dependency increases the complexity of your code and delivers practically nothing. As for the speed boost, assuming it's true does not mean it reduces complexity, faster (initial) dev time very often comes at the cost of creating more complexity.




Join us for AI Startup School this June 16-17 in San Francisco!

Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: