Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Structure #4

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

JonathanILevi
Copy link
Contributor

I totally refactored your code. I was realizing some changes that were not being carried through every repetition of similar code. Also when I was implementing more overloads to serialize and deserialize the number of overloads was growing by powers of 2 to keep every combination available.

As I was coding in this I found a ways to reduce several major repetitions in the code by using some more advanced D meta programming. D's meta programming is uniquely stupid amazing!

However though I reworked your code, the interface for it is the same, all your old unittests were unchanged and still work. I only added and changed the internals.

The I ended up changing the coding style a bit. As I was making the big change I found it easier to look at code familiar to me. If you don't like it and want to have it changed back, I can certainly do that. Though if you do not like the change as a whole, I would rather keep my fork styled my way.

Thank you for making this great library! And making it open source to allow me to expand it to my needs!

Because code coverage reports do not necessarily understand CTFE.
Renamed Grouped to Group.
Refactored code to greatly reduce redundant code.  Compiled code should be the same (if not better).
Added a bunch of unittests to test functionality.
Code is fully backwards compatibility assuming no direct access to internal code was used.
You can now deserialize data to a given instance with classes, structs, and interfaces.
@codecov-io
Copy link

codecov-io commented Nov 24, 2018

Codecov Report

Merging #4 into master will decrease coverage by 1.54%.
The diff coverage is 98.02%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master       #4      +/-   ##
==========================================
- Coverage     100%   98.45%   -1.55%     
==========================================
  Files           2        2              
  Lines         139      259     +120     
==========================================
+ Hits          139      255     +116     
- Misses          0        4       +4
Impacted Files Coverage Δ
src/xserial/attribute.d 50% <50%> (-50%) ⬇️
src/xserial/serial.d 99.21% <98.99%> (-0.79%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 39214ea...10b0f8d. Read the comment docs.

@JonathanILevi
Copy link
Contributor Author

I added several unittests that you can look at to see what I have added.

I added deserializing to preexisting data. Allowing interfaces to be deserialized along with other use cases that tie in nicely with Groups. interfaceTest.deserialize!(EndianType.bigEndian)([...]);

Right now member instances are replaced. But I plan to add a UDA (@WriteTo or @DeserializeTo or a better name) to not replace but deserialize to.

Endian handling is better.  Fully allows either std.system.Endian or xserial.Endian.
Endian enum is used as the UDA.
Any UDA can be given to Serializer including things like Exclude to make exclude default.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants