Re: transformExpr() refactor
От | Neil Conway |
---|---|
Тема | Re: transformExpr() refactor |
Дата | |
Msg-id | 1106025357.22946.134.camel@localhost.localdomain обсуждение исходный текст |
Ответ на | transformExpr() refactor (Neil Conway <neilc@samurai.com>) |
Ответы |
Re: transformExpr() refactor
Refactoring (was: transformExpr() refactor) |
Список | pgsql-patches |
On Thu, 2004-10-28 at 16:49 +1000, Neil Conway wrote: > This patch refactors transformExpr(): rather than being a monsterous 900 > line function, it breaks it up into numerous sub-functions that are > invoked by transformExpr() for individual expression types, in the style > of transformStmt(). I still think this patch is worth applying. Sadly, I will need to basically rewrite it due to code drift. I'm happy to do that, although I'd like to resolve whether we want to accept it or not. Tom and Bruce objected when I posted it originally, although I don't think we reached a conclusion. Why I think the patch is a good idea: 900 line functions are almost universally bad (in fact, I'd be tempted to remove the "almost"). A 900 line function that divides distinct functionality into different branches of a "switch" statement isn't _that_ evil, but it is still a large hunk of code for someone to understand as a single, monolithic unit. Because each branch of the "switch" statement is independent, we can trivially split each branch off into a function -- each branch does something distinct, so it ought to be a distinct function. That means the independence of each branch of the "switch" statement is guaranteed (the function can't modify a local variable in the calling function, for example), and it means that the code is conceptually easier to read. With the present layout, It does mean that you'll need to jump to the function definition if you want to see what a particular branch of the "switch" statement does. But (a) use tags, it's not tough (b) this is a _good_ thing. If I want to understand what the parent function does (transformExpr(), the one with the "switch"), I don't want to have to grok through a 700 line "switch" statement. If each branch of the switch just invokes a function, the intent of transformExpr() is perfectly clear. For refresh everyone's memory, I've attached the original version of the patch. It won't apply cleanly to current sources, but it should apply to HEAD as of approximately Oct. 28, 2004. -Neil
Вложения
В списке pgsql-patches по дате отправления: