andywilsonuk / StringTokenFormatter

Provides string extension methods for replacing tokens within strings (using the format '{name}') with their specified lookup value.
Apache License 2.0
37 stars 1 forks source link

A few changes to discuss #16

Closed TonyValenti closed 5 years ago

TonyValenti commented 5 years ago

Hi Andy, What are your thoughts about the following?

  1. Introduce an editor config that standardizes formatting.
  2. Move TokenReplacer.DefaultMatcher, DefaultFormatter, etc... to TokenMatcher.Default, TokenFormatter.Default, etc.
  3. Introduce a generic FormatToken that calls into the Generic container.
  4. Rename the FormatToken(IDictionary<>) members to FormatDictionary (this is necessary because generics cause FormatToken to take precedence over interfaces.
TonyValenti commented 5 years ago

@andywilsonuk

TonyValenti commented 5 years ago

Also, in a number of the constructors, I'd like to change their signatures a bit and consolidate them down. For example, this:


        public TokenReplacer()
            : this(DefaultMatcher, DefaultMappers, DefaultFormatter)
        {
        }

        public TokenReplacer(ITokenMarkers markers)
            : this(new DefaultTokenMatcher(markers), DefaultMappers, DefaultFormatter)
        {
        }

        public TokenReplacer(IFormatProvider provider)
            : this(DefaultMatcher, DefaultMappers, new FormatProviderValueFormatter(provider))
        {
        }

        public TokenReplacer(ITokenMarkers markers, IFormatProvider provider)
            : this(new DefaultTokenMatcher(markers), DefaultMappers, new FormatProviderValueFormatter(provider))
        {
        }

        public TokenReplacer(ITokenMatcher tokenMatcher, IEnumerable<ITokenToValueMapper> valueMappers, IValueFormatter valueFormatter)
        {
            matcher = tokenMatcher ?? throw new ArgumentNullException(nameof(tokenMatcher));
            if (valueMappers == null) throw new ArgumentNullException(nameof(valueMappers));
            mapper = new TokenToValueCompositeMapper(valueMappers);
            formatter = valueFormatter ?? throw new ArgumentNullException(nameof(valueFormatter));
        }

would essentially become something like this:

        public TokenReplacer(ITokenMatcher tokenMatcher = default, IEnumerable<ITokenToValueMapper> valueMappers = default, IValueFormatter valueFormatter = default) : this(tokenMatcher, new TokenToValueCompositeMapper(valueMappers), valueFormatter) { 

}

        public TokenReplacer(ITokenMatcher tokenMatcher = default, ITokenToValueMapper valueMapper = default, IValueFormatter valueFormatter = default)
        {
            matcher = tokenMatcher ?? TokenMatcher.Default;
            mapper = valueMapper ?? ValueMapper.Default;
            formatter = valueFormatter ?? ValueFormatter.Default;
        }
andywilsonuk commented 5 years ago

Yeah that seems like a good change, the default syntax has really come on in more recent language versions.

TonyValenti commented 5 years ago

@andywilsonuk - Just so you are aware, when I make these kinds of changes, you'll need to release a new major version of the library since it will break binary compatibility with previous versions. I'm OK with that and I think they'll be great changes but I just want to make sure you know the implications.

andywilsonuk commented 5 years ago

good shout, this feels like a major version anyway.

TonyValenti commented 5 years ago

Hi @andywilsonuk - I've got the changes made and I'm working on updating the documentation. I'm going to also put together an "upgrading from v3" doc to cover some of the changes. I'll have everything ready to go in the next few days.

Can I update the metadata info to list myself as an author as well?

TonyValenti commented 5 years ago

@andywilsonuk - Hey Andy, I wanted to let you know I've just created a pull request for a major V4 upgrade: If you try to look at the changes through GitHub, it will suck. I would recommend taking a look at the updated documentation: https://github.com/TonyValenti/StringTokenFormatter/blob/master/README.md

And then download my changes in VS and review them that way. I think you're going to really like the changes I made. It was really awesome getting to work with a codebase that had such a good foundation as well.

andywilsonuk commented 5 years ago

Wow, I'm blown away by your work. I haven't finished looking through it yet but it looks a major upgrade in terms of usability, performance and consistency.

I see you'd added yourself as author which is totally fine by me, I'll add you a collaborator if you'd like so that you can merge in changes without having to create a PR.

I'll need a few more days to finish looking at the changes and then we'll get them merged.

TonyValenti commented 5 years ago

This sounds great! Thanks so much!