3
\$\begingroup\$

I have a list and I want to made some manipulation on it. This my list :

data_two = 
{
    '2021-12-05': [['Mesure 1', {
                'valeur': 0,
                'name': 'User 2'
            }, {
                'valeur': 22.6,
                'name': 'User 1'
            }
        ],['Mesure 2', {
                'valeur': 15,
                'name': 'User 2'
            }, {
                'valeur': 6,
                'name': 'User 1'
            }
        ]]
}

and I want to have something like this

data_two = 
{
    '2021-12-05': [
        ['Mesure 1', {
                'valeur': 0,
                'name': 'User 2'
            }, {
                'valeur': 22.6,
                'name': 'User 1'
            }, 22.6, '7h', ''],
        ['Mesure 2', {
                'valeur': 15,
                'name': 'User 2'
            }, {
                'valeur': 6,
                'name': 'User 1'
            }, 21, '6h', ''],
        ['TOTAL', 15, 28.6, '']
    ]
}

For each items in data_two, I have to make a sum of valeur, so 0+22.6=22.6 and 15+6=21. Then 7h is the index of the max element. The first index is 6h

The element ['TOTAL', 15, 28.6, ''], is the just sum of element grouped by index. So you have 0+15=15 and 22.6+6=28.6

I wrote the code bellow for it

for key, values in data_two.items():
    valeurs_total = []
    total_1 = 0
    total_2 = 0
    for cval in values:
        print(cval)
        maximum = 0
        h = ''
        total_1 += cval[1]['valeur']
        if maximum <= cval[1]['valeur'] and cval[1]['valeur'] > 0:
            maximum = cval[1]['valeur']
            h = '6h'

        total_2 += cval[2]['valeur']
        if maximum <= cval[2]['valeur'] and cval[2]['valeur'] > 0:
            maximum = cval[2]['valeur']
            h = '7h'
        cval.extend([maximum, h, ""])
    maxValue = [total_1, total_2]
    max_value = max(maxValue)
    max_index = '' if max_value == 0 else str(maxValue.index(max_value) + 6) + ' h'
    valeurs_total.extend(['TOTAL', total_1, total_2])
    values.append(valeurs_total)
    data_two[key] = values
    
    
print(data_two)

How can I made this code better ?

New contributor
alexis is a new contributor to this site. Take care in asking for clarification, commenting, and answering. Check out our Code of Conduct.
\$\endgroup\$
5
\$\begingroup\$

Your data structures are a jumble, especially the inner lists. Before worrying about your code, I would encourage you to rethink the data structures. At the outermost layer, you have a dict mapping dates to lists -- so far, that's reasonable. Those outer lists contain inner lists, which consist of a string (eg "Mesure 1") followed by two dicts containing keys valuer and name. Are there always exactly two? Your code implies as much, and I will do the same. In any case, you propose adding even more stuff to the inner lists: a number, a mysterious string (7h or 6h), and an empty string. This kind of data structure doesn't make sense, but without more context it's difficult to give specific advice. If we assume that all of that data should be bundled together, at a minimum you should store it in a more declarative fashion. Here's an illustration of the idea. Since I don't know your project details, focus on the general idea rather than my precise terminology.

# Inner list: a jumble.

[
    "Mesure 1",
    {
        "valeur": 0,
        "name": "User 2"
    },
    {
        "valeur": 22.6,
        "name": "User 1"
    },
    22.6,
    "7h",
    ""
]

# The same data, in a more declarative structure.

{
    "label": "Mesure 1",
    "values": [0, 22.6],
    "total": 22.6,
    "index": "7h",
}

Your code and your desired output disagree. To the list containing "Mesure 2" you say you want to append 21 and 6h, but you actually append 15 and 6h.

Decompose your logic into smaller pieces: extracting values. Complexity is one of the primary enemies in software. One way to keep it at bay is to break things down into simpler, smaller operations. Let's first look at a simpler way to extract the values:

import json

def main():
    values = {
        date : collect_values(outer)
        for date, outer in data_two.items()
    }
    check('values', values)

def collect_values(outer):
    # Takes an outer list. Returns the values, in the same structure.
    return [
        [d['valeur'] for d in inner[1:]]
        for inner in outer
    ]

def check(label, d):
    print(f'\n# {label}')
    print(json.dumps(d, indent = 4))

if __name__ == '__main__':
    main()

Here's the output:

# values
{
    "2021-12-05": [
        [
            0,
            22.6
        ],
        [
            15,
            6
        ]
    ]
}

Decompose your logic into smaller pieces: summaries. An equally small utility function can be used to compute the "summaries" (again, my terminology might be wrong).

def main():
    values = {...}  # Same as before.

    summaries = {
        date : compute_summaries(outer)
        for date, outer in values.items()
    }
    check('summaries', summaries)

def compute_summaries(outer):
    return [
        [v1 + v2, '7h' if v1 < v2 else '6h']
        for v1, v2 in outer
    ]

Decompose your logic into smaller pieces: totals. Same idea.

def main():
    values = {...}     # Same as before.
    summaries = {...}

    totals = {
        date : compute_totals(outer)
        for date, outer in values.items()
    }
    check('totals', totals)

def compute_totals(outer):
    return [sum(vals) for vals in zip(*outer)]
\$\endgroup\$
2
\$\begingroup\$

a down and dirty solution. Itterate the dict and use the key to index the dict. pass that into a function -> compute the return

a more complete solution using list compression

rather than using append you can nest multiple loops into a single line, index the object and pass that into a function to perform some logic operation.

def compute(cval):
    total_1 = 0
    total_2 = 0
    maximum = 0
    h = ''
    total_1 += cval[1]['valeur']
    if maximum <= cval[1]['valeur'] and cval[1]['valeur'] > 0:
        maximum = cval[1]['valeur']
        h = '6h'

    total_2 += cval[2]['valeur']
    if maximum <= cval[2]['valeur'] and cval[2]['valeur'] > 0:
        maximum = cval[2]['valeur']
        h = '7h'
    cval.extend([maximum, h, ""])




x = {key: [compute(cval) for cval in values]
     for key, values in data_two.items()}

print(x)

in terms of readability and maintainability and complexity reduction, putting the logic into a function outside of the loop makes it more sustainable.

\$\endgroup\$
5
  • 5
    \$\begingroup\$ On the Code Review Site our goal is to help the original poster improve their code by making meaningful observations about their code. Alternate solutions with no comments about why they are better than the original code are not considered good answers on code review, although they might be good answers on stack overflow. Please add an explanation of why the code is better, and or add some observations about the original code. \$\endgroup\$
    – pacmaninbw
    Jan 13 at 19:29
  • \$\begingroup\$ I updated my response, with a more complete solution. \$\endgroup\$ Jan 13 at 19:40
  • 1
    \$\begingroup\$ In the future you may want to address how this improves readability and maintainability and reduces complexity which are all important basic software design principles. \$\endgroup\$
    – pacmaninbw
    Jan 13 at 19:45
  • 1
    \$\begingroup\$ Not every python review has to be about readability and maintainability @pacmaninbw - I agree with your first comment but every reviewer is free to focus on what they want, and I think it's good if people try to give what they believe is the most valuable feedback for that particular reviewee(?) instead of just reciting PEP8. \$\endgroup\$
    – ades
    Jan 13 at 20:10
  • 1
    \$\begingroup\$ @ades just trying to help a new user see how they can make insight full observations. I'm not personally a Python programmer. These are also the things I concentrate on when I do a review. I agree with you that every reviewer will provide different insights and that is very valuable on Code Review. \$\endgroup\$
    – pacmaninbw
    Jan 13 at 20:16

Your Answer

alexis is a new contributor. Be nice, and check out our Code of Conduct.

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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