T
T
timur1022018-06-24 09:12:00
Python
timur102, 2018-06-24 09:12:00

Have I correctly divided tasks between functions and how to simplify the code?

Code that can do two things: 0 - print the latest messages and 1 - show 200 messages from the dialog. The code works, but in terms of readability and crutch, I failed. Can it be better to use classes? Am I dividing tasks between functions correctly? Is my architecture correct? In a word: how to improve the code? Didn't answer so I'm posting here.

import vk
from datetime import datetime
import time
import json
import constants # здесь находятся токен и ид пользователя (спец. скрыл, чтоб не узнали)


def auth(): #аутентификация
    access_token = constants.TOKEN
    session = vk.Session(access_token=access_token)
    api = vk.API(session)
    return api

def check_id(api, peer_id): # Кто это? пользователь или группа, узнать имю и фамилию
    if int(peer_id)<0:
        return "Group"
    else:
        time.sleep(0.5) 
        name = api.users.get(user_id = peer_id, fields = "nickname", v="5.80")[0]
        print(name)
        first_name = name['first_name']
        last_name = name['last_name']
        return "{0} {1}".format(first_name, last_name)
def get_Attachments(message, date, dict_ids): #есть ли прикрепления(фото, аудио, документы)
    string = """"""
    if 'attachments' in message.keys():            #не нравится мне это
        for attachment in message['attachments']:  # и это. Может в одну строчку?
            user_id = str(message['from_id'])
            if attachment['type'] == "photo":
                photo = attachment['photo']['sizes']
                for i in photo:
                    if i['type'] == "x":url = i['url']
                string += "{0} photo: {1}  {2}\n".format(dict_ids[user_id], url, date)
            elif attachment['type'] == "doc":
                url = attachment['doc']['url']
                string += "{0} photo: {1}  {2}\n".format(dict_ids[user_id], url, date)
            elif attachment['type'] == "audio":
                url = attachment['audio']['url']
                string += "{0} photo: {1}  {2}\n".format(dict_ids[user_id], url, date)
    return string

def see_msg(api, peer_id):
    dict_ids = {}
    string = """"""
    main_id = constants.main_id
    data = api.messages.getHistory(peer_id=peer_id, count=200, v="5.80")['items']
    data.reverse()
    for message in data:
        date = datetime.fromtimestamp(message['date'])
        date = date.strftime('%Y-%m-%d %H:%M:%S')
        user_id = str(message['from_id'])
        if user_id not in dict_ids.keys():
            dict_ids[user_id] = check_id(api, user_id)
        string += "{0}: {1}  {2}\n".format(dict_ids[user_id], 
                                         message['text'], date)
        attachments = get_Attachments(message, date, dict_ids)
        string += attachments
    return string
def get_msg(api):
    data = api.messages.get(count=200, v="5.80")['items']
    data.reverse()
    print(json.dumps(data, sort_keys=True, indent=4,  ensure_ascii=False))

def main():
    api = auth()
    while 1: # основной цикл
        s = input("0 - get latest msg,\
                   \n1 - See Dialog")
        if s == "0":
            data = get_msg(api)
        elif s == "1":
            peer_id = input("Enter peer_id : ")
            data = see_msg(api, peer_id)
            print(data)

if __name__ == '__main__':
    main()

Answer the question

In order to leave comments, you need to log in

1 answer(s)
A
asd111, 2018-06-24
@timur102

If the code performs two main tasks, then it makes sense to describe only these two functions in the main file, and all auxiliary functions in another file.
If you want with classes, then you can describe the two main functions in the class as public and make the auxiliary ones private.

Didn't find what you were looking for?

Ask your question

Ask a Question

731 491 924 answers to any question