martini-contrib / binding

Martini handler for mapping and validating a raw request into a structure.
MIT License
140 stars 47 forks source link

Additional binding tags and validation features #9

Closed jamieomatthews closed 10 years ago

jamieomatthews commented 10 years ago

After coming from .NET MVC, I am accustom to model binding. However, there were many more validation rules you could set on the model fields.

For example, now you have

 type BlogPost struct {
    Title   string    `form:"title" json:"title" binding:"required"`
}

And you have the "Binding:required", which will add an error if its not there. Is there any chance you'd consider adding some other validation tags, like for example, marking a field as an email? There are some other really common ones too, like credit card, date, minlength, maxlength etc. Sure you could just add your own validator, but it would be nice to be able to add these to the model and have that taken care of for you:

type BlogPost struct {
        Title   string    `form:"title" json:"title" binding:"required" minlength:"10"`
        Email   string    `form:"title" json:"title" binding:"required" email:"true"`
 }

The syntax above is just an example, not necessarily how I would do it. Is this something that might belong here?

mholt commented 10 years ago

That definitely could be useful, and as you come from a .NET MVC background, I understand why.

The thing with adding more and more types of validators into the library is that they won't be used by most people. I mean, maybe; but part of the goal is to keep the package simple. Required-checking seemed pretty necessary across the board, but for specific validations, I think we should leave it up to the user of the package to decide, based on their application's needs.

For instance, what defines a valid email address? Sure, the syntax according to the RFC. But it allows "+" in the username Some applications won't allow "+"-addressing because it can ruin unique email checking as it's still on the same account, even though it's a "valid" email address. Or with a minimum length, is that before or after stripping extraneous whitespace that the user accidentally introduced through a rogue copy+paste? Ultimately, when it comes to deciding what "valid" means, I'd rather not meddle with that. (As an aside, I work in the address validation industry -- yes, there is such a thing -- and defining "valid" is really hard.)

Syntactically, it's a clever idea, but just so you know, I got rebuked by Rob Pike for trying to go against the grain (I didn't know there was a convention). Hence, the tag name is the name of the package, and all its values have to be contained within that tag, rather than adding other tags that aren't package names... by convention, it can't just be a list of properties.

Matt Sherman brought up a similar discussion in golang-nuts and got a similar response from Rob.

I mean, maybe this could work, syntactically:

binding:"required,match=email,minlength=10"

But the complexity of parsing such values adds considerable code to the package. If the community demands it, I suppose we can add it, but I do love simplicity. I'm just afraid of validating things in ways that users won't expect.

jamieomatthews commented 10 years ago

@mholt I actually didn't know that was the standard. Still pretty new here!

I could agree that it would be better to keep it simple. In fact, many ORM's have taken some heat by trying to pack too much functionality into the struct tags (hood, qdb). It can be nice but it definitely comes at a complexity cost. I wish there was a better way, one that could be a bit more strongly typed!

One thing we could do is that we could write some validator func's that would, given an input, validate it on some criteria. So we could write an email validator, credit card, etc. Then people could choose to use it in their validation handlers, or not. This is a compromise to your second point about not stuffing the struct tags, but I guess it would still have the issue of defining what is "valid".

mholt commented 10 years ago

Welcome to Go :+1: As you're seeing I think, it's a great community.

If we wrote a set of validators, they might belong in a separate middleware package entirely, since an extensible set of validators kind of goes beyond just data binding.

If we did go that route, then as you mentioned, we'd have to agree on what is "valid" -- or everyone would be forking and we'd probably end up with a fragmented project. The credit card field is a great example. What is a valid credit card: one that can actually be charged, or a number that clears the checksum? The more logic we impose, the more we have to hope that the users will assume the same things we did.

I'm glad you brought this up. It's a good discussion to have. I'm going to close the issue unless we need to act more on it, but feel free to keep participating and contributing!

jamieomatthews commented 10 years ago

@mholt I agree, a validation package could be cool. Probably wouldnt even be a middleware, but perhaps just a collection of model validations.

Let me think more on this, and see if I can think of a good fit for it within Martini.

jamieomatthews commented 10 years ago

@mholt @codegangsta not sure if this is the right place to put this, but I have thought more on the idea of Validation. I think there could definitely be a place in martini contrib for a Validation package. Here is what I have so far:

Validation would work in harmony with the binding package, as an add-on. Validation has some prebuilt validators, as well as also making it easy for the user to create their own validators (to easily share validation logic between forms/handlers).

The API could work something like this, plugging into a custom validator:

func (bp BlogPost) Validate(errors *binding.Errors, req *http.Request) {

    //init a new validation object, passing a reference to the binding.Errors
    v := Validation{Errors: *errors}

    //run some validators

    v.EmailAdress(bp.UserEmail, "email_address")
    v.MaxLength(len(bp.Content), 60, "content")  //content max length is 60 characters
    v.MinLength(len(bp.Title), 20, "title").Message("This is a custom validation message!")  //title max length is 20 characters
    v.validate() //this will map any validation errors into the binding.Errors object
}

Which will generate a response like this:

fields: {
    title: "This is a custom validation message!",
    content: "Maximum Length is 60",
    email: "Must enter a valid email address"
}

As you can see, each validator would have a default error message. For example, maxLength would be something like

fmt.Sprintf("Maximum Length is %d", max.MaxLength)

But, you can easily override this and provide your own message.

I believe this would make validator methods much cleaner and more readable, as it would be very obvious what was being applied. I have mocked up some of the classes here. I am super open to how it could work though, so I'm looking for some feedback about if and how this could be integrated into martini contrib.

mholt commented 10 years ago

Wow, somehow I missed this; sorry @jamieomatthews. It's been a busy semester! And now here I am at GopherCon finally going through old issues.

I actually like that quite a bit. It's entirely optional and isn't even middleware which kind of clutters up the initialization. It can then be used with discretion on a per-type basis rather than a per-endpoint/handler basis.

If you'd like to round out that package to your satisfaction, I can mention it in the readme here.

(Be aware, though, that with issue #22, the way errors are handled and organized is changing quite significantly.)

attilaolah commented 10 years ago

This looks interesting, except I'd change a few things:

func (bp BlogPost) Validate(errors *binding.Errors, req *http.Request) {
    // Init a new validation object, passing a reference to the binding.Errors and the blog post.
    // In case where BlogPost is a slice type, use NewValidator(errors, bp...) to unpack it.
    v := validation.NewValidator(errors, bp)

    // Run some validators
    v.Key("email_address").Email()
    v.Key("content").TrimSpace().MaxLength(60)
    v.Key("title").TrimSpace().
        MinLength(20, "Title is too short!").
        MaxLength(40, "Title is too long!")
    v.Key("nested").Require().
        Key("objects").Require("This nested object is required!")
    v.Key("array").At(0).Require("First array item is required!")
    v.Key("created_at").Time().Before(time.Now(), "Creation time cannot be in the future!")
    v.Key("website").URL()
    // v.Validate() // I'd rather just leave this out and modify the errors instance in-place during the above calls.
}

Things like TrimSpace would modify the BlogPost instance in-place. If you want to persist such changes, implement Validate with a pointer receiver. If you want to keep the object unchanged, implement Validate on the non-pointer type.

Error messages could be implemented by an optional last string type argument to validators.

Validators could be chained.

This looks a bit challenging to implement, but I think it would be simple enough to use.

jamieomatthews commented 10 years ago

@attilaolah I like some of the modifications you've done. I kind of like the chaining syntax over my method.

Just to get one thing straight, you are preposing that we use reflection to figure out which object in the struct (or nested struct), to validate against? In my initial mockup I was actually manually passing the object in.

mholt commented 10 years ago

@attilaolah What does the string value passed into Key() mean? Is it the field with that JSON or form tag? I also like the chaining idea a lot; but I think I'd rather just pass in the field itself, without the validator needing to use reflection.

attilaolah commented 10 years ago

@mholt I realise now too that passing in the JSON-key to Key() requires some serious reflection, since we would need to get a struct field based on the tag, or fall back to struct field name… Arrgh, not worth it.

I still think though that chaining on a generic matcher object reads better than passing around too many arguments. This would be somewhat similar to the way the gomega matcher works.

To show you what I mean, please have a look at my first try, something I put together just now. Right now I only have At, MinLength, MaxLength, Message, and Classification. Some example usage:

validator.Validator(errors).
    Validate("title", bp.Title).TrimSpace().
        MinLength(10).Message("Title is too short. Minimum 10 characters.").
        MaxLength(100).Classification("too_long").Message("Title is too long. Max 100 characters.").
    Validate("body", bp.Body).
        MaxLength(1000). // default error message will be used
    Validate("tags", bp.Tags).
        MinLength(1).Message("At least one tag is required.").
        At(0). // key is now set to "tags.0"
            MaxLength(100).Message("Tag is too long!")
        // We should also support .Each(), which is like doing .At(i) for every index.

I know, I'm going wild with the chaining here. But I'm just experimenting, and it sure looks fun :) And it was super easy to implement.

You could still use the validator in a non-chained fashion if you wanted. Note that calling validator.Validator(errors) is actually a type conversion, not a function call.

attilaolah commented 10 years ago

Oh, and you can also block on validation errors:

v := validator.Validator(errors)
if v.Validate("items", cart.Items).MinLength(1).Valid() {
    // Valid() returns false if any of the previously chained validators,
    // since the previous Validate() or At() call, result in an invalid state.
    v.Validate("cc", cart.CreditCard).CreditCard() // suppose we add this validator
}
attilaolah commented 10 years ago

In fact, the validator.Validator could also act like an effective "setter" for binding.Errors, in a way:

validator.Validator(errors).
    Invalid("foo").Message("Foo is invalid.").
    Invalid("bar").Classification("no_bar").Message("There really should be a bar at least.")

I'm having fun with this now, so please ignore me if you don't like it. If you do, I can send a PR once I get a few validators added and have them tested properly.

jamieomatthews commented 10 years ago

As referenced at the top of this issue, this package is going to remain separate for now. I like the enthusiasm, I do think this is a really cool idea as well. Let's keep it simple to start, and we can build on features as we go. Chaining must be in the design from the start, though.

attilaolah commented 10 years ago

That's fine, I think too that keeping it separate makes sense. I'll play with it some more and let you know how things are going.

mholt commented 10 years ago

By golly, this is kind of fun to tinker with. There's a lot of different patterns that are possible.

Here's my sketch:

func (bp *BlogPost) Validate(errors binding.Errors, req *http.Request) Errors {
    v := validator.New(errors, req)

    v.Validate(&bp.Title).TrimSpace().Length(10, 100).MustNotContain("foo")
    v.Validate(&bp.Subtitle).TrimSpace().MaxLength(50)
    v.Validate(&bp.Content).TrimSpace().MustContain("Go", "Classification", "Custom message")
    v.Validate(&bp.Author.Email).EmailAddress()
    v.Validate(req).HasHeader("X-Foo-Bar") // maybe not useful; but the idea is you can do validation on the request itself

    // then any other custom validation steps you want to perform yourself,
    // more specific to your application or handler usually

    return v.Errors()
}

Notice:

A couple problems so far unaddressed by any of these solutions:

I like where this is going, but let's not bite off more than we can chew!

attilaolah commented 10 years ago

You would have to pass in the bp pointer somehow to be able to figure out what things like &bp.Title point to, wouldn't you?

Personally, I don't have a problem with specifying Classification and Message separately. But yeah, we could have an Error() method too that would simply replace the last item with the passed-in error, instead of updating its fields. See my approach here (only Classification and Message, but we could easily add Error too.).

Must stop writing now. Running out of battery.

attilaolah commented 10 years ago

Also, Length is nice, but then you can only specify one error message with the chained Message method. For example, you can't do .MinLength(10).Message("foo").MaxLength(100).Message("bar") with just .Length(10, 100). I guess that's fine though.

Or maybe you could .Length(10, 100).Message("too short", "too long")? That sounds quite easy to do in fact. Just range through the arguments in Message.

attilaolah commented 10 years ago

Also, instead of

v.Validate(req).HasHeader("X-Foo-Bar")

I'd probably do

v.Header("X-Foo-Bar").Require()

or something similar, since you already have the request, so no need to pass it in.

The idea is that Header(string) gives you a matcher, just like Validate(), so you can do stuff like v.Header("Foo-Bar").Len(10, 100).

attilaolah commented 10 years ago

And for more generic matchers, I'd just provide a .Match(regexp.Regexp). That gives you power :)

Also, TrimSpace could be useful in combination with something like Truncate(length).

mholt commented 10 years ago

@attilaolah No, I don't think the bp object actually needs to be passed in at all, because each validator step only works on primitives such as string, int, etc. For example, a MustContain function would work on strings. Fortunately we can do type assertions and type switches easily without reflection in order to implement this.

// For example, for a validator that can perform some check
// on both strings and ints:

if val, ok := value.(int); ok {
    // ... validate an integer
} else if val, ok := value.(string); ok {
    // ... validate a string
} else {
   panic("Squeak! Don't know how to validate that type")
}

That brings up another interesting question: if the type of the object passed into Validate is not supported, what happens? Does it fail silently? How are errors reported? Does it just panic and issue a 500 to the client? (It's not a bad idea per-se, since the programmer definitely made an error if that's the case.)

You're right about the Header thing; my example wasn't very well thought-out.

I look forward to seeing how this ends up!

jamieomatthews commented 10 years ago

@mholt I agree, I think 500 is better than failing silently. If anything, I think 500 should be the default, and if it makes sense to do so, handlers can choose to fail silently. I definitely like your syntax as posted in your previous answer...looks concise, yet readable. I'm mocking it up now

attilaolah commented 10 years ago

@mholt yeah, we don't need reflection to do the validations. However, we do need to reflect on the json tags if you want to avoid passing in the FieldNames.

Which is why I would simply pass in the field name.

jamieomatthews commented 10 years ago

@attilaolah in the example i've started to write, I have 1 constructor that just takes a pointer to a field, and one that will take a pointer, and the key value. That way the user can decide if he wants to just have it reflected, or if he wants to manually set it. That way the user still has full control

attilaolah commented 10 years ago

@jamieomatthews ok, that sounds cool — but can you reflect on a pointer to a field? I didn't know you can do that. I thought you would have to reflect on the struct itself to get the tag. I thought dereferencing a pointer to a field of the struct would simply point to the value, which would only give you type information and the value, but not the tags.

I don't have much experience with reflection though, so I am probably mistaken.

As for failing with a 500, I think too that it sounds reasonable. It would probably mean a programming mistake. Things like Length, MinLength, MaxLegth shold consult, say, a private length() method, that would look something like:

if val, ok := value.(string); ok {
    return len(val)
} else if val, ok := value.([]interface{}); ok {
    return len(val)
} else if val, ok := value.(map[interface{}]interface{}); ok {
    return len(val)
} else {
   panic(fmt.Sprintf("object type %T has no length", value))
}
mholt commented 10 years ago

@jamieomatthews You're right; that's currently what's being done now anyway. I'm hoping to keep the names of the fields in just one place, and that's in the struct tags. (If people need to squeeze out every hertz of performance they can, they shouldn't be using Martini in the first place.) Spreading the field names across the struct definition and in validation sounds like it could get messy pretty quick.

Another thought I had: it'd be cool if this validation package wasn't constrained to just martini-contrib/binding. In order to generalize this, all we'd have to do is abstract away the Errors and Error type into an interface. What do you think? Then any Go web service could use the validator.

jamieomatthews commented 10 years ago

@mholt There is no real reason why this has to apply only to Martini. I was starting to feel, in fact, that we might want an internal representation of the errors anyway (not directly take binding.Errors). Which, at that point, we might as well make it generalized.

If we did that, we'd probably make the default output a json string, but we could also provide an adapter for the martini-bind package.

mholt commented 10 years ago

@jamieomatthews Since interfaces are satisfied implicitly, you could define one in the Validation package, like:

type Errors interface {
    Has(string) bool
    HasField(string) bool
    Add([]string, string, string)
    Get(int)
}

type Error interface {
    Fields() []string
    Classification() string
    Message() string
}

(And better yet, could we build on the native Go error type?)

This way any consumer of the Validation package could use their own error types if they want... or the built-in Validation ones; it's up to them.

rselbach commented 10 years ago

You might want to check out https://github.com/go-validator/validator which does these kinds of validations. We wrote it for a project exactly to be able to deal with incoming JSON/XML and have been using it for a few months.

It has a pluggable validation rule system and allows for different rule sets for different scenarios for the same struct.

If you need an email validator, you write one and then simply

validator.SetValidationFunc("email", myEmailValidator)

and then you can have them in your structs

type User struct {
    Username string `validate:"nonzero"`
    Email string      `validate:"email"`
    // other fields
}

You get the idea. And then you can have different rules for different scenarios, maybe when creating a new user, you need everything, but when it's some change, you don't need the email to be filled, so you can do

type User struct {
    Username string `create:"nonzero" update:"nonzero"`
    Email string      `create:"email"`
    // other fields
}

... 
u := User{Username: "gopher"}

// invalid as Email is not a valid email
valid, errs := validator.WithTag("create").Validate(u)

// valid since we don't care for email and Username is filled
valid, errs = validator.WithTag("update").Validate(u)
mholt commented 10 years ago

@robteix I think it'd definitely be good to try and share code here, maybe even combine these ideas with your project.

To be honest, my personal preference is to keep things out of the struct field tags when possible, and put logic in code, like we're doing here. But you already have several good validators written that we could build from. There's definitely strengths to both efforts so far.

rselbach commented 10 years ago

We initially wrote something similar to JSONschema but it was most trouble than it solved. The rationale for attaching the rules to the structs themselves was to allow us to replace 30 lines of validation for large structs with a single function call. That said, I can see how the approach is not to everybody's taste.

It still hasn't happened for me, but I do fear we may eventually get to a point where we're going to have a ton of different rules in a struct and that will be ugly.

This package does have a helper function that allows you to run the validation without struct tags by passing the rules as a parameter --

if valid, errs := validator.Valid(myVariable, "min=10,max=100,regexp=^[a-z]+$"); !valid {
    // freak out and treat the errors :)
}

To be honest, I don't love this syntax as it looks inelegant to me, but maybe it can be a start.

jamieomatthews commented 10 years ago

Ok all, I have put some work into mocking this up, and I have a few things I want input on:

func (contactRequest ContactRequest) Validate(errors binding.Errors, req *http.Request) binding.Errors {

    v := validation.New(errors, req)

    // //run some validators
    v.Validate(contactRequest.FullName, "full_name").MaxLength(20)
    v.Validate(contactRequest.Email, "email").Default("Custom Email Validation Message").Classify("email-class").Email()
    v.Validate(contactRequest.Comments, "comments").TrimSpace().MinLength(10)

    return v.Errors
}

Here is a sample of what I have currently working. Here are a couple things to notice:

@mholt, in your example above, you wrote this example

.MustContain("Go", "Classification", "Custom message")

Does this mean each validator would take a value, and then a variable amount of strings, and assume the first one is the classification, and the second param is the error message? Which would essentially allow you to leave them out if you wanted?

The last thing I was thinking is that it would be nice to specify that you only want a single error for a given field. Maybe a .SingleError() at the end which would only map the first error received? In a lot of cases you probably have some priority to your errors, so you could just chain them in order of most important, to least important.

Thoughts?

attilaolah commented 10 years ago
  1. I wouldn't special-case MustContain() to accept classification and error message as variable number of arguments. That way you throw away some nice compilation checks. I would simply force users to use .Classify() and .Default().
  2. You can probably allow .Classify() and .Default()` to come after the validators/modifiers, since they simply modify the message. If you've already written the error, you could just modify the error in-place.
  3. Instead of using a .SingleError(), maybe you could inject a nop-validator that blocks the validation chain if there is an error. Something like:
v.Validate(contactRequest.Email, "email").Email().IfValid().MustContain("@foo.com")

I'm just toying with the idea… basically, if the .Email() validation fails, the IfValid() would return a nop-object that does not add any further errors.

However, that might be hard to explain. I'd rather just make i texplicit:

if s := v.Validate(contactRequest.Email, "email").Email(); s.Valid() {
    s.MustContain("@foo.com").Default("Only emails from the foo.com domain are allowed.")
}
mholt commented 10 years ago

@attilaolah Yes, you're right -- the extra arguments into a validation step are not ideal, so let's drop that and go with something like what you're saying (referring to your no. 1).

To be really nit-picky at this point, having .Classify() and .Default() (or I kind of like .Message() which is slightly less ambiguous IMO) can make for really long chains. Would it be better to pass in some struct instead:

v.Validate(contactRequest.Email).Next(validation.Options{
    Classification: "LengthError",
    Message: "Length is too short because of this or that",
}).MinLength(5).MustContain("baz")

Or something like that maybe? It might be more verbose but I think it would be easier to read and implement. Hopefully you wouldn't be setting a custom error message and classification for each validation step, since that would get ugly fast... (but I think it would either way, even if using .Classify() and .Default().)

I'm glad that the field name won't have to be specified in validation since it is available by some relatively simple reflection by looking at struct tags. (If struct tags weren't specified for some reason, maybe case-insensitive field name matching would do, like what encoding/json does).

Let's also see what we can glean from the work that @robteix has done; I've been taking a look at his repo, and it's an interesting approach. (Though, I don't think a single string with validation rules would be powerful enough while staying readable, and he has acknowledged that possible downside. I think we're finding a good middle ground.)

If verbosity/line-length is becoming long, what about defining a bunch of validation rules at once like in a slice or struct or map instead of chaining them? I'll keep playing with ideas in the meantime, but I think we're really making progress toward a general-purpose solution.

attilaolah commented 10 years ago

@mholt you mean validation.Options, not v.Options, right?

And I also agree that it would be nice if we could somehow incorporate @robteix's work, instead of re-doing everything from scratch. Thanks @robteix for joining the discussion :)

mholt commented 10 years ago

@attilaolah Yup, thanks. Fixed.

Another nit-pick: package name... validation or validator? Or just validate?

jamieomatthews commented 10 years ago

I'm very flexible on the name. I'd say the repo name should probably match the master object that you create (right now we create v := validation object).

I do like the name validator for the overall object, but when I wrote this earlier, I already had an interface called validator (meets the go standard of adding 'r' to the end of an interface). This defines the criteria for the individual validators. I could switch that interface to be ValidatorStep or something though.

Also, @mholt I agree Message() is a better name for the method. Do you like Classify() to set Classification? Its a bit shorter than Classification(), which I like.

@attilaolah those are some good suggestions on how to do some sort of conditional validations. I think I prefer the second, more explicit solution

attilaolah commented 10 years ago

@jamieomatthews yeah, I like the second one too.

Regarding that. It would be interesting if your initial call to Validator() would return the same interface as all the return values of the individual methods on that object. In that case, you could simply write:

v = validation.New(…)
if v = v.Validate(contactRequest.Email, "email").Email(); v.Valid() {
    v.MustContain("@foo.com").Default("Only emails from the foo.com domain are allowed.")
}

without creating an intermediate variable. You would have to save the state between method calls anyway for the chaining to work. At this point, we probably don't even need to re-assign it:

if v.Validate(contactRequest.Email, "email").Email().Valid() {
    v.MustContain("@foo.com").Default("Only emails from the foo.com domain are allowed.")
}

Just mutating the original object should be enough… Although in that case, it is less obvious that it actually acts on the same value. Plus, if you .Validate() a different value inside an if-block, that would have side effects after the block.

But I guess people would realise that it comes with chaining and not do such things :)

mholt commented 10 years ago

@jamieomatthews You make a good point about running out of names. :) I'm not quite sure what to do about that yet, personally. I trust that you and Attila will figure out a good solution.

And yes, I do like .Classify() over .Classification(). It's shorter and just as clear.

jamieomatthews commented 10 years ago

@mholt, in reference to the errors interface you stubbed out above, I need to clarify a few things

type Errors interface { Has(string) bool HasField(string) bool Add([]string, string, string) Get(int) }

Has(string) - what does this method do?
Get(int) returns object at an index? If we have this, we probably want a length method as well

mholt commented 10 years ago

Hey @jamieomatthews, you're right, that initial draft wasn't very clear. I imagine the one over in this comment would be more useful. What do you think?

jamieomatthews commented 10 years ago

@mholt

Ah, I didnt see this. This looks much more specific. Heres what I have so far. Do you think ForClass would be better, to match ForField, instead of just Class()?

type Errors interface {
    Class(classification string) Errors
    ForField(name string) Errors
    Get(class, fieldName string) Errors
    Add(fieldNames []string, classification string, message string)
}
mholt commented 10 years ago

Yeah, that's looking good!

And how about WithClass? This method is kind of annoying because it's hard to name what you call the type or kind of error it is... I figured Classification (Class) would be the least ambiguous/overloaded compared to Kind/Type/etc. I'm down for WithClass though if you are.

jamieomatthews commented 10 years ago

I'm good with WithClass

I just finished the internal implementation, the only other field I needed was something along the lines of HasErrors(). So this could be either accomplished with a Count() (or NumErrors()) or simply a HasErrors()

mholt commented 10 years ago

Good point. With the concrete implementation right now, it's a simple len(errors) but with the interface, we need a method, huh. I like Count().

attilaolah commented 10 years ago

I like Count() too, although I generally like to name that method Len(), for the single lazy reason that it already implements one third of sort.Interface.

mholt commented 10 years ago

@attilaolah Has a good point, I didn't even consider that! I'll let you pick, @jamieomatthews -- I'm good with either. Thanks for all your hard work during a busy time!

mholt commented 10 years ago

We've made major progress in the last couple weeks, and commit e20ce50c10d6e02d9db60e055c0600bb208b8653 is the first fruits of our efforts. The binding package should now be ready to be used in conjunction with your validation package, @jamieomatthews! I might have missed something, so let me know, but AFAIK this is looking good. Thanks for the discussion @attilaolah!

Closing this issue, I think we're just about ready to move forward with RC 1.

scottkf commented 10 years ago

@mholt @jamieomatthews @attilaolah thanks for your work on these issues!