Closed rahl0b10 closed 6 years ago
Hi @rahl0b10.
There's a lot to like here:
That said, it's not quite there yet.
For me, the most fundamental concern is that the new functionality has been built into the new builder. This breaks the equivalence of the two APIs. What I like about the addition of the builder class is that it provides choice, but at the moment that choice is limited because the new features are only supported in one of the two APIs.
I don't think there's any fundamental reason why access via property names or column reordering couldn't be offered via the old-style syntax. Is there any way we could move it into the core and provide access to it via the builder class? In other words, I suppose I'm asking if we could simplify things by using the builder class as a wrapper around the core API?
Some other (more minor) feedback:
DemoAnimals
in there for now.)To reiterate, I think you're on to a winner here. We just need to iron out a few of the details. As ever, thanks for your contributions!
Tom
PS Are @geekmdio another of your accounts?
@tdwright - Thanks for all the feedback :) Those are great points, and I will implement in that way. I appreciate the concept of equivalency of the API's. I hadn't really considered that. Even thought not a direct hit in terms of backward compatibility, it indirectly forces people to refactor if they want the new functionality. It'll be quite a bit more work, so give me a bit of time.
As for the @geekmdio account, yes it's mine. I'm working on something proprietary under that account. It's all in it's infancy. It's showing up because OS X is doing something strange when I push. The push goes to the correct account, but under the wrong username no matter what I do with the local and global settings. I found out that if I push via HTTPS I get the desired behavior, my local git config is respected, and the correct account is credited with the push. Not a huge inconvenience, but still annoying.
Great stuff! And don't sweat it if it takes a while - we're doing this in our free time, remember? 😉
I'll leave this functionality to you, but I may do some stuff elsewhere, so be sure to merge the develop back into your repo from time to time.
Ha, ha. Yes, good point. In our free time. ;)
I'm a self-taught amateur (who works in an entirely different profession), so to date I haven't worked in a team environment. This is as close as I've been so far, so I may be over-eager at times.
Hi @rahl0b10
Hope you had a good Christmas / winter break?
Were you still hoping to make the changes we discussed? I may have some capacity coming up in the next few weeks, so could start work on some aspects of this, if you've not already started? Don't want to tread on your toes!
Tom
@tdwright - Hi! Yes, Christmas was good. Busy, but good. Hope yours was as well! Anyway, I've been very busy lately. I would like to continue contributing, but if you get to a feature before I do I will not feel offended. Its your project anyway :) Please feel free to do as you please.
Think we should probably close this one now.
Feature #20
Use a builder pattern for table creation.
This proposed solution provides two improvements in the API.
Table<T>
.NOTE:
Build()
returns Table<T>.Feature #8
Allow columns to be arbitrarily ordered
This solution was also integrated into the builder pattern. This allows for a very streamlined table construction process. Simply passing a preferred index and the name of the target
Column
allows for the position of the targetColumn
and theColumn
occupying the destination index to be swapped.Example class
Default ordering of columns (based on relative property position).
New ordering of columns after
IntCol
was explicitly set to location with index of 0.Chainable
Because these two features are part of the same builder class, they can be chained.
HideColumn(name)
andSetColumnIndex(name, index)
methods can be chained in a highly streamlined table creation process, for example: