4 ms·

... into 1000-ish lines in python and some xml dialects spread over 20ish files that may or may not do the same thing... covered by 5 tests when there are clear

by amichal 11y ago

... into 1000-ish lines in python and some xml dialects spread over 20ish files that may or may not do the same thing... covered by 5 tests when there are clearly more then 5 initial states driving the original if.... I would have left it as the big if. Source: https://github.com/groupserver/gs.group.member.canpost https://github.com/groupserver/gs.group.member.canpost

orf 11y ago
I thought the same[1]. I've never really used any of the zope Python stuff, it seems to all be very java-esque. I guess it used to be popular (maybe still is in a niche) and if it works then great, but damn its ugly compared to more pythonic code. 1. https://github.com/groupserver/gs.group.member.canpost/blob/master/gs/group/member/canpost/configure.zcml https://github.com/groupserver/gs.group.member.canpost/blob/...
wyldfire 11y ago
> 1000-ish lines in python ... Well, an if/elif/elif block that spans 300 lines is often much more difficult to comprehend than 100 - ten line functions (especially if all 100 share the same function signature). That said, I'm not sure if I prefer the xml configuration option. But the design is overall likely more flexible now than it was before.
- amichal 11y ago
  If the flexibility was desirable then yes but it by definition makes the code do more then it did before and is therefore harder to really understand. In this concrete case, these many functions are now spread over the entire codebase (spanning many repos). In many classes and inheritance trees and driven by a ton of configuration so there are now many many more possibilities. Here is one implementation https://github.com/groupserver/gs.group.type.discussion/blob/master/gs/group/type/discussion/requiredpropertiescanpostrule.py#L95-L116 https://github.com/groupserver/gs.group.type.discussion/blob... which i don't find very readable personally Reading the README in the first repo I can now see the flexibility and coupling the rules more closely to their objects was a design goal. So in fairness the new code is doing a lot more then (what i imagine to be) old code to be doing. Also I now note this code is 3+ years old so the zope dependency (which i know nothing about) makes some more sense.

by amichal 11y ago

orf 11y ago

I thought the same[1]. I've never really used any of the zope Python stuff, it seems to all be very java-esque. I guess it used to be popular (maybe still is in a niche) and if it works then great, but damn its ugly compared to more pythonic code. 1. https://github.com/groupserver/gs.group.member.canpost/blob/master/gs/group/member/canpost/configure.zcml https://github.com/groupserver/gs.group.member.canpost/blob/...

wyldfire 11y ago

> 1000-ish lines in python ... Well, an if/elif/elif block that spans 300 lines is often much more difficult to comprehend than 100 - ten line functions (especially if all 100 share the same function signature). That said, I'm not sure if I prefer the xml configuration option. But the design is overall likely more flexible now than it was before.

amichal 11y ago
If the flexibility was desirable then yes but it by definition makes the code do more then it did before and is therefore harder to really understand. In this concrete case, these many functions are now spread over the entire codebase (spanning many repos). In many classes and inheritance trees and driven by a ton of configuration so there are now many many more possibilities. Here is one implementation https://github.com/groupserver/gs.group.type.discussion/blob/master/gs/group/type/discussion/requiredpropertiescanpostrule.py#L95-L116 https://github.com/groupserver/gs.group.type.discussion/blob... which i don't find very readable personally Reading the README in the first repo I can now see the flexibility and coupling the rules more closely to their objects was a design goal. So in fairness the new code is doing a lot more then (what i imagine to be) old code to be doing. Also I now note this code is 3+ years old so the zope dependency (which i know nothing about) makes some more sense.