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

Miscellaneous #22

Open
perazz opened this issue May 30, 2024 · 1 comment · Fixed by #33
Open

Miscellaneous #22

perazz opened this issue May 30, 2024 · 1 comment · Fixed by #33

Comments

@perazz
Copy link

perazz commented May 30, 2024

List of miscellaneous improvements that I'm seeing on the current repository

  • wherever possible, define pure elemental functions:

fig/src/fig_rgb.f90

Lines 15 to 23 in 36c15fa

function rgb_to_int(color) result(rgb_int)
type(RGB), intent(in) :: color
integer(pixel) :: rgb_int
rgb_int = ior(ishft(color%a, rgb_bit_depth*3),&
ior(ishft(color%b, rgb_bit_depth*2),&
ior(ishft(color%g, rgb_bit_depth), color%r)))
end function rgb_to_int

  • Functions that are type initializers should go in an initializer interface:

fig/src/fig_rgb.f90

Lines 25 to 32 in 36c15fa

function int_to_rgb(rgb_int) result(color)
type(RGB):: color
integer(pixel), intent(in):: rgb_int
color%a = ibits(rgb_int, 3*rgb_bit_depth, rgb_bit_depth)
color%b = ibits(rgb_int, 2*rgb_bit_depth, rgb_bit_depth)
color%g = ibits(rgb_int, rgb_bit_depth , rgb_bit_depth)
color%r = ibits(rgb_int, 0, rgb_bit_depth)
end function int_to_rgb

i.e. something like

interface RGB
   module procedure int_to_rgb
end interface

! So you can define your RGBs as 
my_color = RGB(pixel_int)
  • Use i0 format to avoid spaces when printing integers:

write(color_string, '(A,I3,A,I3,A,I3,A,F5.3,A)') 'rgba(', color%r, ',', color%g, ',', color%b, ',', alpha, ')'

  • Put class functions inside the derived type:

    fig/src/fig_rgb.f90

    Lines 5 to 11 in 36c15fa

    type :: RGB
    sequence
    integer(rgb_level) :: r
    integer(rgb_level) :: g
    integer(rgb_level) :: b
    integer(rgb_level) :: a
    end type RGB
type(mytype)
...
contains
   procedure, non_overridable :: to_int => rgb_to_int
end type

cc @everythingfunctional @johandweber

@AnonMiraj AnonMiraj mentioned this issue Jun 3, 2024
@everythingfunctional
Copy link

  • Put class functions inside the derived type:

I'm actually starting to move away from this unless polymorphism is actually going to be used. It feels like a nicer syntax and less namespace pollution, but I'm less and less convinced it's worth it to couple to a language/design feature that you aren't actually taking advantage of.

The rest I pretty well agree with.

@AnonMiraj AnonMiraj linked a pull request Jun 12, 2024 that will close this issue
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 a pull request may close this issue.

2 participants