Skip to content

Conversation

@Pharap
Copy link
Owner

@Pharap Pharap commented Mar 26, 2021

This will finally allow SFixed and UFixed to be converted to and from each other.

As simple as this implementation seems, it took quite a bit of thought to come up with it.

Early on I realised that I could implement the function by leveraging the 'scaling' cast operator to perform the scaling and then having a '1-to-1' operator that simply converts the internal representation of the same width by effectively leveraging the built-in integer cast operators, but the real problem was figuring out how to have those two cases coexist. Originally I went down the wrong track by trying to create a specialisation, and at one point I was considering using a template class and some specialisation, but then I suddenly realised that I could introduce a non-template cast operator to do the job of the non-scaling/direct operation and suddenly the rest just slotted into place.

It took me quite some time to actually commit this because I wanted to double check that I'd got the order of operations correct. As it happens, I had got it right first time, so I ended up wasting a lot of time creating the code to check when I could have committed it straight away, but at least I know for definite.

All in all I'm glad I put this particular issue off for a few years because my original attempt from 2018 was absolutely awful (I think perhaps I was following some bad advice I got from StackOverflow) and I feel that I've been able to pull off something much better than my original intended implementation.

Resolves #20

This will finally allow SFixed and UFixed to be converted to and from each other
@Pharap Pharap added Minor This change is a minor addition Improvement This change improves the code's behaviour labels Mar 26, 2021
@Pharap Pharap self-assigned this Mar 26, 2021
@Pharap Pharap added this to the v1.1.0 milestone Mar 26, 2021
@Pharap Pharap merged commit ea71949 into master Mar 26, 2021
@Pharap Pharap deleted the add-sign-casting-functionality branch March 26, 2021 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Improvement This change improves the code's behaviour Minor This change is a minor addition

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add conversions between SFixed types and UFixed types

2 participants