데이터를 변경하는 인스턴스 메서드의 save() 포함 여부

[Kirade]

데이터를 변경하는 인스턴스 메서드의 save() 포함 여부에 관한 의견을 들어보고 싶습니다.

아래의 Post모델과 인스턴스 메서드 publish가 있다고 가정하겠습니다.

class Post(models.Model):
    published_at = models.DateTimeField()
    
    def publish(self):
        # Implemented by Option 1 or Option 2
        pass

인스턴스 메서드의 save() 함수의 포함 여부에 따라 다음과 같이 두 가지 방식으로 함수를 사용할 수 있습니다.

Option 1

def publish(self):
    self.published_at = timezone.now()

def somewhere(post):
    post.publish()
    post.save()

Option 2

def publish(self):
    self.published_at = timezone.now()
    self.save()

def somewhere(post):
    post.publish()

제 생각에는 Option 2 의 방식이 대부분의 경우 합리적인것 같습니다.
일반적으로 publish 라는 행위에서 실제로 포스팅이 공개되는 것 까지를 기대하게 될 것 같기 때문입니다.

만약 Option 1 형태로 작성할 경우가 종종 필요할 수 있는데요.
이 경우 명시적으로 함수의 이름을 set_published_at 과 같은 형태로 만들어줄 수 있을것 같습니다.

다른 분들의 의견이 궁금해요.

[donghyun0218]

저도 두 번째 방법을 더 선호합니다.

도메인 지식을 다루는 도메인 객체와 DB 에 직접 질의를 하는 ORM 이 분리가 되어있다면 첫 번째 방법을 선호할 것 같지만,
저희는 특별히 도메인 객체와 ORM 을 구분해서 다루지 않으니, 저는 두 번째 방법을 선호합니다.

[leehosung]

저도 optione2에 동의합니다.
예제는 아마 self.published_at = timezone.now() 의 형태겠죠? self.save() 로 이어지는.

[Kirade]

넵

[anohk]

Option 2에 동의합니다.
DB 변경없이 인스턴스의 상태만 변경하는 경우에는, 말씀하신대로 set_ 을 붙여주는 것이 좋은 것 같아요.

[twins1040]

저도 Option 2 에 동의합니다.

다만 만약 publish, set_published_at 둘다 만들어야 하는 상황이라면,
set_published_at 을 만들지 않고

def publish(self, commit=True):
    self.published_at = timezone.now()
    
    if commit is True:
        self.save()

와 같이 만드는 것을 선호합니다.
publish 함수의 로직을 한곳에 둘 수 있고, django 에서 이미 commit 인자를 널리 사용하고 있어서 의미 전달이 잘 됩니다.

[donghyun0218]

이 방법에 동의합니다.

[anohk]

commit 인자를 사용하는 방식에 동의합니다.

[Kirade]

저는 Option 2 의 의미가 commit=False 를 사용하는 경우도 있지만 그렇지 않은 경우도 있을것이라 봅니다.
User 의 set_password 가 그런경우 같구요.

이 논의는 필요시 다른 Discussion 에서 진행하도록 해요~

[Kirade]

#13