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

[IMPL] Add implicit conversion from RECT -> System.Drawing.Rectangle. #388

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AraHaan
Copy link

@AraHaan AraHaan commented Mar 2, 2024

Proposal Implementation

Adds an implicit conversion operator from RECT -> Rectangle with an explicit operator from Rectangle back to RECT when the rectangle needs converted to an RECT for calls into Windows APIs that requires it. Also with operators handling these conversions programmers would worry less on if they got their conversion between RECT <-> Rectangle correct or not by eliminating all the guess work from it. Unit tests of the operators will come soon. As such I opened this pull request as a draft.

Implements/Fixes #387.

Fixes terrafx#387.

Signed-off-by: AraHaan <seandhunt_7@yahoo.com>
Comment on lines +31 to +32
public static implicit operator Rectangle(RECT rectangle)
=> Rectangle.FromLTRB(rectangle.left, rectangle.top, rectangle.right, rectangle.bottom);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't safe to be implicit, as it potentially overflows and thus is lossy.

Comment on lines +35 to +36
=> new
{
Copy link
Member

@tannergooding tannergooding Mar 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: target typed new is one of my least favorite language features and I'm pretty sure it's blocked in the editorconfig, so I expect this will cause CI to fail.

@tannergooding
Copy link
Member

Same general feedback as given on the issue. I'm not particularly a fan of this, as its potentially lossy and brings in a new dependency.

This really seems like something where a user could provide extension methods for their own library to solve the need.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants