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
[Discussion]: Column buckling calculations in continuum mechanics module #17072
Comments
You should consider whether the class needs to be mutable or not. If you can make a non-mutable class which takes everything it needs in the constructor, I'd recommend that. This is different than the Beam class. I also don't immediately see the value of have Beam and Column be derived from the same class. The only thing they share are a cross section and material properties. |
Yeah, you can skip these but don't forget to mention the conventions used in the docstring of the class. Not sure but won't it be better if we make |
What's wrong with making a new object? If I was designing the Beam class from scratch I wouldn't bother with any of the setters and getters. I would add all properties in the Another point about code design is that the Beam class has some methods that probably shouldn't be there. For example
There are also cases like All of this happens because more and more contributors arrive wanting to add new methods without considering the overall design. It's not possible to remove those methods from Beam now (although the code can certainly be simplified). It's important to design a clear API so that people aren't tempted to keep adding unnecessary things to it. Avoiding mutability is a big simplification that I would make from the start. |
Is there any specific reason why we are intending to make column class non-mutable? I think it would be good if I mention it somewhere in the blogs or in any wiki page. |
If I'm not wrong it is basically to reduce the number of lines of code spent in setters and other methods to modify the properties of the object and thus making the class simpler. |
@ishanaj it is really up to you to think what is best here. Do you see any benefit in making it mutable/immutable? I suggested making it immutable because that's (usually) simpler. Most things are immutable in SymPy which is useful for caching etc. Matrix is an example where allowing mutability has lead to many problems that are now impossible to fix without breaking backwards compatibility. Your class is relatively isolated though so those concerns probably don't apply. Another point is that you need to think about consistency with Beam. Unfortunately users will compare these two so you should consider whether they will have any particular reason for wanting this class to be mutable. Another important point: from a backwards compatibility perspective it is always possible to change your mind and add mutability later but not the other way around. |
As we plan to implement column buckling calculations in the continuum mechanics module, here is what I have thought of regarding the API and its implementations:
The API can be seen from the stage -II of the proposal here
I think we need some discussions regarding how the API looks and how it would work.
Since the beam calculations are different from those of Column, we would require a different class which probably would be a subclass of
Beam
.Some initial questions:
Are there any other end-conditions?
it, to make the code extensible to it in future.
@jashan498 @moorepants @oscarbenjamin
The text was updated successfully, but these errors were encountered: