2
\$\begingroup\$

Suggestions to better understand when and how is convenient to use OPP.

This question is a follow-up question on this post.

I have a script that reads an input file. In time, the script may be modified to account for new elements, that are directly added in the same input file. I would like to better understand how OOP paradigm can help to simplify this updating procedure.

The code is written based on the suggestions from @Peilonrayz.

Input file:

SIMPLY SUPPORTED BEAM
NNODES<NNnodes><Node,X,Y>
2
1,0,0
2,1,0
SECTIONS<NSec><Sec,Area,Inertia,Depth,ShearCF>
1
1,100000,1,1,0

from __future__ import annotations
from dataclasses import dataclass

@dataclass
class Mesh:
    title: str
    nnode: NNode
    sections: Section

    @classmethod
    def load(cls, file):
        return cls(
            file.readline(),
            NNode.load(file),
            Section.load(file)
        )

_NNODE_KEYS = ['ID', 'x', 'y']
@dataclass
class NNode:
    nodes: List[dict]

    @classmethod
    def load(cls, file):
        file.seek(0)
        while not file.readline().startswith('NNODES'):
            continue
        amount = int(file.readline())
        values = []
        for node in range(amount):
            values.append(dict(zip(
                _NNODE_KEYS,
                file.readline().split(',')
            )))
        return cls(values)


_SECTIONS_KEYS = ['Sec', 'Area', 'Inertia','Depth','ShearCF']
@dataclass
class Section :
    sections: List[dict]

    @classmethod
    def load(cls, file):
        file.seek(0)
        while not file.readline().startswith('SECTIONS'):
            continue
        amount = int(file.readline())
        values = []
        for node in range(amount):
            values.append(dict(zip(
                _SECTIONS_KEYS,
                file.readline().split(',')
            )))
        return cls(values)

Next, if the script is modified to handle more data, one can use inheritance.

SIMPLY SUPPORTED BEAM
NNODES<NNnodes><Node,X,Y>
2
1,0,0
2,1,0
SECTIONS<NSec><Sec,Area,Inertia,Depth,ShearCF>
1
1,100000,1,1,0
MATERIALS<NMat><Mat,Young,Poisson,Thermal,Weight>
1
1,30000000,0.3,0,0

from __future__ import annotations
from dataclasses import dataclass

@dataclass
class Mesh:
    title: str
    nnode: NNode
    sections: Section

    @classmethod
    def load(cls, file):
        return cls(
            file.readline(),
            NNode.load(file),
            Section.load(file)
        )

_NNODE_KEYS = ['ID', 'x', 'y']
@dataclass
class NNode:
    nodes: List[dict]

    @classmethod
    def load(cls, file):
        file.seek(0)
        while not file.readline().startswith('NNODES'):
            continue
        amount = int(file.readline())
        values = []
        for node in range(amount):
            values.append(dict(zip(
                _NNODE_KEYS,
                file.readline().split(',')
            )))
        return cls(values)


_SECTIONS_KEYS = ['Sec', 'Area', 'Inertia','Depth','ShearCF']
@dataclass
class Section :
    sections: List[dict]

    @classmethod
    def load(cls, file):
        file.seek(0)
        while not file.readline().startswith('SECTIONS'):
            continue
        amount = int(file.readline())
        values = []
        for node in range(amount):
            values.append(dict(zip(
                _SECTIONS_KEYS,
                file.readline().split(',')
            )))
        return cls(values)

@dataclass
class Mesh_mat(Mesh):
    materials: Material

    @classmethod
    def load(cls, file):
        return cls( file.readline(),
                   NNode.load(file),
                   Section.load(file),
                   Material.load(file)
                   )

_MATERIAL_KEYS = ['Mat', 'Young', 'Poisson','Thermal','Weight']
@dataclass
class Material :
    materials: List[dict]

    @classmethod
    def load(cls, file):
        file.seek(0)
        while not file.readline().startswith('MATERIALS'):
            continue
        amount = int(file.readline())
        values = []
        for node in range(amount):
            values.append(dict(zip(
                _MATERIAL_KEYS,
                file.readline().split(',')
            )))
        return cls(values)

Anyway, I don't see any convenience in using this approach instead of simply modify the Mesh class.

from __future__ import annotations
from dataclasses import dataclass

@dataclass
class Mesh:
    title: str
    nnode: NNode
    sections: Section
    materials: Material

    @classmethod
    def load(cls, file):
        return cls(
            file.readline(),
            NNode.load(file),
            Section.load(file),
            Material.load(file)
        )

_NNODE_KEYS = ['ID', 'x', 'y']
@dataclass
class NNode:
    nodes: List[dict]

    @classmethod
    def load(cls, file):
        file.seek(0)
        while not file.readline().startswith('NNODES'):
            continue
        amount = int(file.readline())
        values = []
        for node in range(amount):
            values.append(dict(zip(
                _NNODE_KEYS,
                file.readline().split(',')
            )))
        return cls(values)


_SECTIONS_KEYS = ['Sec', 'Area', 'Inertia','Depth','ShearCF']
@dataclass
class Section :
    sections: List[dict]

    @classmethod
    def load(cls, file):
        file.seek(0)
        while not file.readline().startswith('SECTIONS'):
            continue
        amount = int(file.readline())
        values = []
        for node in range(amount):
            values.append(dict(zip(
                _SECTIONS_KEYS,
                file.readline().split(',')
            )))
        return cls(values)

_MATERIAL_KEYS = ['Mat', 'Young', 'Poisson','Thermal','Weight']
@dataclass
class Material :
    materials: List[dict]

    @classmethod
    def load(cls, file):
        file.seek(0)
        while not file.readline().startswith('MATERIALS'):
            continue
        amount = int(file.readline())
        values = []
        for node in range(amount):
            values.append(dict(zip(
                _MATERIAL_KEYS,
                file.readline().split(',')
            )))
        return cls(values)

Since I don't have experience in OPP programming, which of these two approaches is better, if a different person who writes the code is asked to add modifications(like handles more data) ?

The code can be further improved as @Peilonrayz said in the first question (while cycle and controls over the input data) but I would stay focused only on OOP improvements

Improve this question
\$\endgroup\$
0

1 Answer 1

Reset to default
3
\$\begingroup\$

Now that we have more classes we can start utilizing more OOP. All the classe NNode, Section and Material have the load class method. This means we can utilize inheritance to simplify the code using inheritance

class LoadCSV:
    _FLAG: ClassVar[str] = ''
    _KEYS: ClassVar[Tuple[str, ...]] = ()

    @classmethod
    def load(cls, file):
        file.seek(0)
        while not file.readline().startswith(cls._FLAG):
            continue
        amount = int(file.readline())
        values = []
        for node in range(amount):
            values.append(dict(zip(
                cls._KEYS,
                file.readline().split(',')
            )))
        return cls(values)


@dataclass
class NNode(LoadCSV):
    _FLAG = 'NNODES'
    _KEYS = ('ID', 'x', 'y')

    nodes: List[dict]


@dataclass
class Section(LoadCSV):
    _FLAG = 'SECTION'
    _KEYS = ('Sec', 'Area', 'Inertia','Depth','ShearCF')

    sections: List[dict]


@dataclass
class Material(LoadCSV):
    _FLAG = 'MATERIALS'
    _KEYS = ('Mat', 'Young', 'Poisson','Thermal','Weight')

    materials: List[dict]

Next, if the script is modified to handle more data, one can use inheritance.

Anyway, I don't see any convenience in using this approach instead of simply modify the Mesh class.

The code can be further improved as @Peilonrayz said in the first question (while cycle and controls over the input data) but I would stay focused only on OOP improvements

No inheritance there sucks, much like the original Mesh class. Both of them are bad. You can see that they're bad, so you should give up on this OOP quest, and actually implement something of substance. Because OOP is a means to organize code not actually do something. "Doing something in OOP" may sound like OOPs doing all the work, but people should really say "Doing something utilizing OOP".

You have gotten all the OOP improvements you can get. Since you only want OOP improvements, I can do no more. Please further improve the code.

  1. Change Mesh, with the three loads, so that it can work on any of the provided files. If NNODES is there, or if it's not.

    If you make this change in LoadCSV.load then all of the other objects will automatically benifit from it.

  2. Remove _KEYS from the classes. Your file provides them, and what you're doing is unOOP like and just janky.
Improve this answer
\$\endgroup\$
2
  • \$\begingroup\$ "Because OOP is a means to organize code not actually do something" is one of the key concept I was missing. The LoadCSV class is still a dataclass? P.S. I want only OOP improvements because I want to figure out the other problems by myself. \$\endgroup\$
    – Stefano
    Feb 18, 2020 at 10:19
  • 1
    \$\begingroup\$ @Stefano "The LoadCSV class is stall a dataclass?" No, because it doesn't have @dataclass before it. Also it doesn't make sense to make it a dataclass, as it has no attributes. It only has class variables, which probably wouldn't play ball if you make it a dataclass. \$\endgroup\$
    – Peilonrayz ♦
    Feb 18, 2020 at 10:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.