4 ms·

rather than point-by-point rebuttal as the sibling requests, I think this sums up the coding style pretty well: https://github.com/seleniumbase/SeleniumBase/blo

rather than point-by-point rebuttal as the sibling requests, I think this sums up the coding style pretty well: https://github.com/seleniumbase/SeleniumBase/blob/v4.33.11/seleniumbase/core/capabilities_parser.py#L29-L173 https://github.com/seleniumbase/SeleniumBase/blob/v4.33.11/s...

harrall 2y ago
That's not amazing code but that's not that bad. In the grand scheme of things, that's not code debt that would ever seriously make my life any harder.
- TeMPOraL 2y ago
  Yup. At least it's self-contained and easy to step through and modify if something breaks or needs to be changed. And, a my previous PM would point out, even the copy-pasting and verifying no mistakes were made was a solution that took a fraction of the time a modern "clean" approach would. She had a point; as much as I'm against writing this simple code in the general case, plenty of devs tend to err towards overcomplicating solutions when given a chance. I mean, the modern, proper, Clean Code™ solution would have this split into multiple files (not counting tests), and across two or three abstraction levels. I've seen this happen enough that I can tell I'd much prefer working with code like this capabilities parser (and hell, it can be beaten into near-perfection in an hour or three).
  - the_real_cher 2y ago
    Amen! I think the more experienced you get in coding the more you appreciate straight forward code you can immediately look at and understand.
seleniumbase 2y ago
That method came from code that I accepted in a PR from December 31, 2019: https://github.com/seleniumbase/SeleniumBase/pull/459 https://github.com/seleniumbase/SeleniumBase/pull/459 Not a true representation of most of the code today.
- parineum 2y ago
  The code is in the code base. Presumably, it still gets run. It doesn't make a difference if new code doesn't look like that.
  - seleniumbase 2y ago
    Call it "legacy code" if you'd like. That specific part is from a less common feature for setting options when running on a Selenium Grid. The new CDP Mode isn't compatible with The Grid (since CDP Mode makes direct CDP API calls without making Selenium API calls).
    - MstWntd 2y ago
      it's always easier for people today to look at the work of other people in the past and draw stupid conclusions.. don't mind them..
  - wisty 2y ago
    Bad old code has been battle tested. Bad new code has not, and is more likely to have the show stopper bugs you want to avoid.
    - seleniumbase 2y ago
      There's actually a lot of examples being used for testing (https://github.com/seleniumbase/SeleniumBase/tree/master/examples https://github.com/seleniumbase/SeleniumBase/tree/master/exa...), which are run regularly (locally and in GitHub Actions). Plus, a lot of major companies are using SeleniumBase: https://github.com/seleniumbase/SeleniumBase/blob/master/help_docs/happy_customers.md https://github.com/seleniumbase/SeleniumBase/blob/master/hel... (if something breaks, I find out quickly)
- the_real_cher 2y ago
  It's not really bad thought. It's clear, it's intuitive, it's easy to understand on first glance, it's a single purpose function, it's easy to step through. you don't have anything to defend here.

harrall 2y ago

That's not amazing code but that's not that bad. In the grand scheme of things, that's not code debt that would ever seriously make my life any harder.

TeMPOraL 2y ago
Yup. At least it's self-contained and easy to step through and modify if something breaks or needs to be changed. And, a my previous PM would point out, even the copy-pasting and verifying no mistakes were made was a solution that took a fraction of the time a modern "clean" approach would. She had a point; as much as I'm against writing this simple code in the general case, plenty of devs tend to err towards overcomplicating solutions when given a chance. I mean, the modern, proper, Clean Code™ solution would have this split into multiple files (not counting tests), and across two or three abstraction levels. I've seen this happen enough that I can tell I'd much prefer working with code like this capabilities parser (and hell, it can be beaten into near-perfection in an hour or three).
- the_real_cher 2y ago
  Amen! I think the more experienced you get in coding the more you appreciate straight forward code you can immediately look at and understand.

seleniumbase 2y ago

That method came from code that I accepted in a PR from December 31, 2019: https://github.com/seleniumbase/SeleniumBase/pull/459 https://github.com/seleniumbase/SeleniumBase/pull/459 Not a true representation of most of the code today.

parineum 2y ago
The code is in the code base. Presumably, it still gets run. It doesn't make a difference if new code doesn't look like that.
- seleniumbase 2y ago
  Call it "legacy code" if you'd like. That specific part is from a less common feature for setting options when running on a Selenium Grid. The new CDP Mode isn't compatible with The Grid (since CDP Mode makes direct CDP API calls without making Selenium API calls).
  - MstWntd 2y ago
    it's always easier for people today to look at the work of other people in the past and draw stupid conclusions.. don't mind them..
- wisty 2y ago
  Bad old code has been battle tested. Bad new code has not, and is more likely to have the show stopper bugs you want to avoid.
  - seleniumbase 2y ago
    There's actually a lot of examples being used for testing (https://github.com/seleniumbase/SeleniumBase/tree/master/examples https://github.com/seleniumbase/SeleniumBase/tree/master/exa...), which are run regularly (locally and in GitHub Actions). Plus, a lot of major companies are using SeleniumBase: https://github.com/seleniumbase/SeleniumBase/blob/master/help_docs/happy_customers.md https://github.com/seleniumbase/SeleniumBase/blob/master/hel... (if something breaks, I find out quickly)
the_real_cher 2y ago
It's not really bad thought. It's clear, it's intuitive, it's easy to understand on first glance, it's a single purpose function, it's easy to step through. you don't have anything to defend here.