Review, suggestion etc?

[Michał Jaworski]

I've made a quick look at the code and even executed it. It looks pretty
clear and is easy to understand, although it has some structural problems.
I won't do a thorough review but highlight the most important problems.

First, the recursive user input pattern you use:
      def marriage():
        maritals = input('Married: Yes/No ?: ').title()
        while maritals != 'Yes' and maritals != 'No':
            return marriage()
        return maritals

It may look really convenient but you should really avoid that. In small
amounts it can be harmless, but it is kind of a resource leak. Aslo this
pattern is omnipresent so can quickly get out of hand and exceed maximum
recursion depth:

   ...
   You can choose only numbers.
   What you wanna do?:
   You can choose only numbers.
   What you wanna do?:
   You can choose only numbers.
   What you wanna do?:
   You can choose only numbers.
   What you wanna do?:
   You can choose only numbers.
   What you wanna do?:
   You can choose only numbers.
   Fatal Python error: Cannot recover from stack overflow.

   Current thread 0x0000000112836dc0 (most recent call first):
     ...
     File "vimart.py", line 52 in menu_choice
     File "vimart.py", line 52 in menu_choice
     ...
   Abort trap: 6

Also, this choice-prompt pattern could be moved to a separate general
function so you don't have to repeat it all over the code. Take a look at
the prompt() function from click.termui module to get a better
understanding on how to build such things (
https://github.com/pallets/click/blob/master/src/click/termui.py). I would
also recommend creating helper functions for common things like displaying
results because right now it's hard to figure out which parts of
application constitute its presentation layer and which are part of "core
logic". Try to keep these two concepts separate and you will get better
results.

Second major structural "problem" are inline function definitions:

def add_people():
    def how_old():
        ...

    def p_sex():
        ...

    def marriage():
        ...

You shouldn't be doing that unless you need a closure with nonlocal
variables to read from. Otherwise it really harms the readability. I
understand the urge to keep relevant code close to the usage but you would
have better results with modules. If you really want to keep everything in
a single module, keep functions close together in the file but don't nest
them. Use relevant naming conventions instead. Simply think of how you
would write that in C. You could use some object-oriented approach too, but
I would recommend polishing structural programming first.

There are also other issues like not always closing files, and not
validating the user input in specific places. Still, these will be more
evident as you improve the structure of application and do thorough
testing. Anyway, the code doesn't look bad. It of course needs improvement
but I've also seen worse specimens from people being actually paid for
writing the code.

Remember that "how to make things better" is a function of few parameters:
- what you want to achieve
- what are you able to do
- how much time do you have

If your sole goal is to learn and improve skills, I would recommend
polishing this in current procedural fashion to some extent, but not trying
to make it perfect. Then I would start from scratch and experiment with
different paradigms (OOP for instance). Then I would try to make a smaller
project that has some actual utility.

czw., 17 gru 2020 o 04:17 Bischoop <Bischoop at vimart.net> napisał(a):

> On 2020-12-17, Bischoop <Bischoop at vimart.net> wrote:
>
>
> Accidently removed the paste, https://bpa.st/E3FQ
> --
> https://mail.python.org/mailman/listinfo/python-list
>