I’m working on a project that needs lots of toolbars on screen at once, even though not all of them will be used at the same time. So, I’m modelling this ‘foldable’ dock widget after what I remember Photoshop panels used to be like.

It’s a work in progress, but would like to hear constructive suggestions.

https://blocks.programming.dev/0101100101/42c5d67f86c049baa3500aa38e439f8a

  • FizzyOrange@programming.dev
    link
    fedilink
    arrow-up
    3
    ·
    4 days ago

    Easily above average code for Python. I’m going to pick on one method:

    def _set_float_icon(self, is_floating: bool):
                """ set the float icon depending on the status of the parent dock widget """
                if is_floating:
                    self.float_button.setIcon(self.icon_dock)
                else:
                    self.float_button.setIcon(self.icon_float)
    

    First, Python does have ternary expressions so you can

    self.float_button.setIcon(self.icon_dock if is_floating else self.icon_float)
    

    Second, what does this code do?

    foo._set_float_icon(true)
    

    Kind of surprising that it sets the icon to icon_dock right? There are two easy fixes:

    1. Use *, is_floating: bool so you have to name the parameter when you call it.
    2. I’d probably rename it to _update_float_icon() or something.

    Also use Black or Ruff to auto-format your code (it’s pretty well formatted already but those will still improve it and for zero effort).

    • 0101100101@programming.devOP
      link
      fedilink
      English
      arrow-up
      2
      ·
      1 hour ago

      Thanks for the compliment! Python isn’t my first language and it’s difficult to be able to switch style from one language to another!

      I always find it difficult to choose when to use ternary statements. Sometimes, for something quick and simple, I will, otherwise I’ll be explicit. This is more of a readability issue than anything else. And I find the ternary statements quite verbose compared to other languages by using the words if/else rather than shorthand symbols.

      You’re absolutely right about the set_float_icon and corresponding method. Coding’s an iterative process and that’s a byproduct. I think set_float_icon() along with complimentary methods like set_docked_icon(), set_minimize_icon(), set_restore_icon() etc may be easier to use / remember wtf it does in six months time!

      Thanks for the black / ruff suggestion. I’ve never heard fo them, but I’m about to go look for them.

    • logging_strict@programming.dev
      link
      fedilink
      arrow-up
      1
      ·
      4 days ago

      ternary expressions

      Recommend against the ternary expression. coverage might not detect the two code blocks. Would also not be able to apply # pragma: no cover comment to a code block that isn’t important enough to justify testing it. Often use do nothing code blocks.

      self.float_button.setIcon(self.icon_dock if is_floating else self.icon_float) lets say while testing something goes wrong and trying to debug what happened.

      Will not see the value that gets past into self.float_button.setIcon

      I like your idea of having the param be keyword only. Makes it more readable.

      • FizzyOrange@programming.dev
        link
        fedilink
        arrow-up
        1
        ·
        3 days ago

        If you branch coverage tool can’t handle branches on the same line I would suggest you use a different one! Does it handle if foo or bar?

        Will not see the value that gets past into self.float_button.setIcon

        Uhm, yes you will? Just step into the function.

        • logging_strict@programming.dev
          link
          fedilink
          arrow-up
          1
          ·
          3 days ago

          self.float_button.setIcon is a Python wrapper around a C++ library. Might not be possible to, or want to, step into the function.

          if foo or bar is part of a if-else condition. The else portion needs # pragma: no cover or coverage may complain.

          • FizzyOrange@programming.dev
            link
            fedilink
            arrow-up
            2
            ·
            1 day ago

            Well… the else condition (bar) needs to be covered. I haven’t used branch coverage tools in Python but in any sane language you cover the actual semantics, not the lines. It shouldn’t make any difference if you write your code on one line, or with ternary expressions, or whatever.

            • logging_strict@programming.dev
              link
              fedilink
              arrow-up
              1
              ·
              1 day ago

              What matters is what the package coverage can do and what limitations or nuances it has. Have to work with what we have. We are lucky to have coverage. Especially within a subprocess and multiprocessing workers

            • logging_strict@programming.dev
              link
              fedilink
              arrow-up
              1
              ·
              1 day ago

              We are dealing with Python and coverage, not some theoretical situation or circumstances.

              Was assuming measuring branch coverage. In which case, my advice is coming from experience