KnowSheet / Sherlock

Structured, append-only, immutable data persistence layer with publish-subscribe.
2 stars 1 forks source link

First real shot at polymorphic Yoda. #22

Closed dkorolev closed 9 years ago

dkorolev commented 9 years ago

Hi @mzhurovich ,

First real shot at polymorphic Yoda.

Done:

Remains:

I have a couple more topics to chat of in my tape recorder. Hope to find time to go through them before we chat tomorrow morning.

Talk soon and would appreciate it if you take a look.

Thanks! Dima

dkorolev commented 9 years ago

@mzhurovich : PTAL.

Polymorphic-friendly syntax done. It behaves when when only one type is supplied, but I ran into two troubles with more than one at the same time.

As of trouble 1, I don't see of now whether it's possible to support arbitrary methods from various YodaImpl-s via inheritance. The cleanest possible syntax I can think of right now is along the lines of `api.Magic<KeyEntry>::SomeMethod(...). I'll sleep over it.

As of trouble 2, it has two sub-troubles: one closely related to trouble 1 (inheritance + virtual functions don't do what they should), and another one with diamond-shaped hierarchies.

I don't particularly worry about trouble 2 since we can always make the syntax of visitors a bit uglier (ex. REGISTER_VISITABLE()). Trouble 1 certainly requires at least a sleepover.

In any case: PTAL at the files split and at the branched out test. Thanks!

PS: Could you please also look through https://github.com/KnowSheet/Bricks/pull/153 and https://github.com/KnowSheet/Bricks/pull/154? Thanks!

dkorolev commented 9 years ago

@mzhurovich :two_men_holding_hands:

Done :innocent:, ready for :scream:, feels :city_sunrise:.

I'll go grab some dinner. Won't do more Sidewire tonight, so might as well take a fresh look at this code before we sync up.

Good morning :sunrise: and looking forward to syncing up before I take off for the night.

Thanks, Dima

dkorolev commented 9 years ago

And the matrix code is in:

[==========] Running 4 tests from 3 test cases.
[----------] Global test environment set-up.
[----------] 2 tests from YodaKeyEntry
[ RUN      ] YodaKeyEntry.SmokeImpl
[       OK ] YodaKeyEntry.SmokeImpl (26 ms)
[ RUN      ] YodaKeyEntry.Smoke
[       OK ] YodaKeyEntry.Smoke (2 ms)
[----------] 2 tests from YodaKeyEntry (28 ms total)

[----------] 1 test from YodaMatrixEntry
[ RUN      ] YodaMatrixEntry.Smoke
[       OK ] YodaMatrixEntry.Smoke (2 ms)
[----------] 1 test from YodaMatrixEntry (2 ms total)

[----------] 1 test from Yoda
[ RUN      ] Yoda.CoverTest
[       OK ] Yoda.CoverTest (1 ms)
[----------] 1 test from Yoda (1 ms total)

[----------] Global test environment tear-down
[==========] 4 tests from 3 test cases ran. (31 ms total)
[  PASSED  ] 4 tests.
dkorolev commented 9 years ago

@mzhurovich : And your matrix code and test are in, passing.

It's ugly in a few places (sorry, I'm about to sleep soon). Worth touching to my taste:

  1. File-wise and class/namespace-wise location of Exception-s. (Why not keep them within KeyEntry<> and MatrixEntry<>?)
  2. File-wise and class/namespace-wise location of typedefs such as T_KEY (Why not keep them within KeyEntry<> and MatrixEntry<>?)
  3. File-wise and class/namespace-wise location of helpers such as key extractor (I'd suggest we keep them local to the same file that defines KeyEntry, MatrixEntry, etc.)
  4. Container vs. ActualContainer: our logic is again spread out, where Container is a, hmm, Container+Mutator. Some structure won't hurt.

@mzhurovich : I'm online for another ~20 minutes, and in ~10 hours.

Thanks! Dima

mzhurovich commented 9 years ago

@dkorolev It was really great pull request! Merging to move on.