0

I have the following class that creates some comment and save it to dynamo db that must contain unique comment id. How to "implement" counting - a simple counter that will create the unique id for a comment. I don't want to repeat rsu_id value.

class RsuService(object):
    next_id = 0

    def __init__(self):
        self.rsu_id = RsuService.next_id
        RsuService.next_id += 1

    async def create(self, alert_id, text):
        message = {
            'event_id': alert_id,
            'text': text,
            'is_rsu': True,
            'comment_id': self.rsu_id
        }
        await save(message)

Is it good implementation? How to improve it?

  • 2
    Depends on how unique you want it. For example, when restarting the program, it will start over from 0. – CristiFati Nov 14 '17 at 11:45
  • Is there any way to avoid this? –  Nov 14 '17 at 11:46
  • 1
    You can use GUID/UUID for this, refer this https://stackoverflow.com/questions/534839/how-to-create-a-guid-uuid-in-python – Himanshu Bansal Nov 14 '17 at 11:49
  • 1
    if the id does not have to be incremental use can use UUID as few here suggested. Otherwise you can query the DB for the highest ID when loading and update your counter accordingly. – AndreyF Nov 14 '17 at 11:51

2 Answers2

0

I think this is not good a approach. You can generate UUID for each comment and use it as unique id

import uuid

class RsuService(object):
    async def create(self, alert_id, text):
        message = {
            'event_id': alert_id,
            'text': text,
            'is_rsu': True,
            'comment_id': uuid.uuid4().hex
        }
        await save(message)
ryche
  • 2,004
  • 2
  • 18
  • 27
0

I think it is a good implementation, if you don't have cases of concurrent operations with RsuService class.

But it may fail in case two or more objects of the RsuService are created at the same time. Beacuse += operations is not atomic in python.

I would suggest you to this if you have case of concurrent operations.

import threading
class RsuService(object):
    next_id = 0
    lock = threading.Lock() #lock shared by all objects of this class

def __init__(self):
    lock.acquire()
    self.rsu_id = RsuService.next_id
    RsuService.next_id += 1
    lock.release()

And if you don't have cases of concurrent tasks, it would be better to use timestamp as unique id, because if you restart the program your counter will start from beginning which will be an issue.

import time
...
RsuService.next_id = time.time()
Abhijith Asokan
  • 1,865
  • 10
  • 14