From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 082744414C; Wed, 5 Jun 2024 15:01:02 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C1A64402DC; Wed, 5 Jun 2024 15:01:01 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mails.dpdk.org (Postfix) with ESMTP id 124D340289 for ; Wed, 5 Jun 2024 15:01:00 +0200 (CEST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id DC83F339; Wed, 5 Jun 2024 06:01:23 -0700 (PDT) Received: from [10.57.40.142] (unknown [10.57.40.142]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id BBD6D3F792; Wed, 5 Jun 2024 06:00:58 -0700 (PDT) Message-ID: Date: Wed, 5 Jun 2024 14:00:57 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 3/5] dts: add parsing utility module Content-Language: en-GB To: =?UTF-8?Q?Juraj_Linke=C5=A1?= , dev@dpdk.org Cc: Jeremy Spewock , Paul Szczepanek References: <20240412111136.3470304-1-luca.vizzarro@arm.com> <20240509112635.1170557-1-luca.vizzarro@arm.com> <20240509112635.1170557-4-luca.vizzarro@arm.com> <685b9f60-7b5d-407f-be26-69115e23a9d2@pantheon.tech> From: Luca Vizzarro In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 05/06/2024 13:19, Juraj Linkeš wrote: >>>> +        return cls.compose(partial(int, base=int_base), >>>> cls.find(pattern)) >>>> + >>>> +    """============ END PARSER FUNCTIONS ============""" >>>> + >>>> +    @classmethod >>>> +    def parse(cls, text: str) -> Self: >>> >>> Would converting this into __init__(self, text: str) work? Sounds >>> like we could just use "for field in fields(self)" and then setattr() >>> to populate the variables. >> >> I am not in favour of overriding the constructor, as I see the parsing >> ability as an extension to the class. > > Well, the class should primarily do the parsing (based on its name), so > everything else is an extension in my mind >> Nonetheless, this would make sense if we think that this would be used >> exclusively for objects that need to be parsed in order to be >> constructed. I purposefully left the flexibility open, but if we don't >> think it'll ever be needed, then it's not a game changer to me. > > Everything about the class indicates (or outright says) that parsing > should be the only purpose. I can't really imagine what else we could > add (in this state, an instance before calling parse() would be just the > text with fields containing function in the metadata needed for parsing > - what else is there to do with this data?). Can you elaborate if you > can think of something? > > And again, that said, it doesn't matter much, but I like the constructor > as it really doesn't look like the class could (or should) do anything > else than parsing text. I think there is some confusion going on. The class is any data object like TestPmdPort. TextParser is an implementation of a parsing functionality, a Protocol/interface with a pre-implemented functionality, which can be added as an extension. The only way this would make sense is if these data objects are actually "text representation to object adapters". If we are exclusively interpreting them as such, then yes, it would totally make sense to bring a custom constructor. But if we will need to re-use the objects for other reasons aside from parsing, then we may have to revert to the current implementation.