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

Create the PhpMyAdmin\ShapeFile\ShapeType class #26

Merged
merged 1 commit into from
Sep 14, 2023

Conversation

MauricioFauth
Copy link
Member

@MauricioFauth MauricioFauth commented Sep 14, 2023

Replaces the hard-coded shape type integers with class constants.

I'm not sure if, as a next step, ShapeType should be a value object or an enumeration or leave it as is.

@codecov
Copy link

codecov bot commented Sep 14, 2023

Codecov Report

Merging #26 (55cc5e4) into master (ab4a803) will decrease coverage by 0.52%.
The diff coverage is 100.00%.

Additional details and impacted files
@@             Coverage Diff              @@
##             master      #26      +/-   ##
============================================
- Coverage     92.22%   91.70%   -0.52%     
  Complexity      250      250              
============================================
  Files             3        4       +1     
  Lines           656      615      -41     
============================================
- Hits            605      564      -41     
  Misses           51       51              

Replaces the hard-coded shape type integers with class constants.

Signed-off-by: Maurício Meneghini Fauth <mauricio@fauth.dev>
@MauricioFauth MauricioFauth merged commit f7d512e into phpmyadmin:master Sep 14, 2023
13 checks passed
@MauricioFauth MauricioFauth deleted the shape-type branch September 14, 2023 22:37
@MauricioFauth MauricioFauth self-assigned this Sep 14, 2023
@MauricioFauth MauricioFauth added this to the 4.0.0 milestone Sep 14, 2023
@kamil-tekiela
Copy link
Contributor

Why did you not use an enum?

@MauricioFauth
Copy link
Member Author

MauricioFauth commented Sep 19, 2023

Why did you not use an enum?

I thought about it at first, but I noticed that the shape type integer is not limited to those ones, so I used constants to keep the int type for now.
So, I'm not sure how to handle the unknown values with an enum.
The specification says that shape type is int<0, 33>.

Shape types not specified above (2, 4, 6, etc., and up to 33) are reserved for future use.

Should we use an enum or a value object?

@kamil-tekiela
Copy link
Contributor

I think we cannot accept unknown types. We should limit the shape type to only the types listed in the specification.

@MauricioFauth
Copy link
Member Author

I think we cannot accept unknown types. We should limit the shape type to only the types listed in the specification.

Makes sense. I opened an issue for this. #29

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.

3 participants