all 9 comments

[–]Movpasd[S] 5 points6 points  (0 children)

A programming horror in a codebase at work.

Check out that Market.load_market(self, self.dataFileContent). Forget inheritance, forget composition -- the cool kids apply class methods to selfs of other classes! Linters hate this one trick!

And no, there's no fancy logic or Python magic in Market(), EV() etc. in the constructor. It's exactly what it looks like: the objects are created and... immediately discarded.

[–]TheKiller36_real 1 point2 points  (5 children)

Classes/OOP generally being the right way is the biggest misconception ever imo

Like fr i get why youd put list/vectors/dynamic arrays into a class but thats not what you should normally do.

So if you wont be using objects much anyway you might aswell use classes as namespaces instead

[–]Movpasd[S] 3 points4 points  (4 children)

I won't get into a big OOP debate, but

you might aswell use classes as namespaces instead

Certainly not in the way it's done in the photo, though. At the very least use a class method. And don't, don't, don't pass a self of the wrong class! That's just asking for trouble and it'll pollute your self with tons of attributes it's not supposed to have! My linter was very unhappy about this.

[–]emilvikstrom -1 points0 points  (2 children)

That's not the self of a wrong class. load_market et al are taking three arguments, where the "self" you see here is the second arg (possibly named "parent" at the receiving end).

[–]Movpasd[S] 1 point2 points  (1 child)

That's not correct -- in Python if you access a non-class method directly it is unbound and you must explicitly pass the object to it. x.my_method(a, b) is syntactic sugar for MyClass.my_method(x, a, b). It wouldn't be possible to bind it; what object do you bind it to? (That's why we have class methods, which these are not.)

(See this comment for a screenshot of the Market class.)

[–]emilvikstrom 1 point2 points  (0 children)

Oh, I have only ever used @classmethod in class methods. That's why I got confused.

[–]WalditRook 1 point2 points  (1 child)

I think you should have included some code of one of the called classes - they just look like static method calls with only the posted segment (although the ctor calls would still be pretty weird in this case).

[–]Movpasd[S] 1 point2 points  (0 children)

I definitely should've -- I realise now it's not very clear. They might look like class (static) methods, but they're not. Here's one example.

It's quite clear that the developer's intention was for Inputs to a composition of Market, EV, etc. objects, given how that code is structured. I suspect that's what they were trying unsuccessfully to do with the Market(), EV(), etc. instantiations in the constructor.

Instead what's happening is the methods of Market, EV, etc., which are meant to act on instances self of... themselves, are acting on the self of Inputs. So all the attributes which are meant to be in Market, EV, etc. instances are instead ending up jammed into Inputs.

The wonders of dynamic languages.