On Sun, 2005-07-03 at 22:34 +0200, smurfd wrote: > Im abit confused, so does this all mean that the patch is okey as it is. > Or does it mean it's not okey?!
The original is better than the last one or this one. The problem is when discussing code there's always a lot of ways to do the same thing. Maybe it is an experience thing, and it isn't always good - I often see so many ways to solve even simple problems that I'm blinded from the more straightforward ones. So in email or irc what comes out is often a rush of partially thought-out possibilities, rarely the ideal path, and sometimes not even a reasonable path. Normally that process is iteratively repeated quickly in your head and you just cull the less ideal solutions and then the more reasonable solutions will just bubble to the surface, usually based on a compromise of various competing variables. e.g. abstraction vs efficiency, space vs speed, expression vs conciseness. But if you're in the middle of something else you dont really have the time to stop your head, put it in a different gear for anothe problem someone has asked you about, think about it properly, then switch over again afterwards. So I apologise if the various discussions just get you confused, they're often thought experiments which aren't meant to go further, rather than actual solutions. > something like this perhaps then, i had to re-use the size_to_string() > function aswell, since it was a private one too. Eek no, definitely not. Having duplicated structures around is definitely a recipe for disaster. Its also full of c99 stuff like in-line variable declarations which are right out, and it still leaks the output of size_to_string (at least). Ugh, who on earth wrote all that 1e3L mess, hmm, messy. Whats so unreadable about "1024". *shakes head* But I digress ... I have an easier solution: Convert the size_to_string in to a public static method on e-msg-composer-attachment(or -bar) (i.e. just make it an external function, say e_msg_composer_attachment_size_to_string). You dont need to change anything but its name (this is more or less what you had in the first patch right?). Add a single public method to e-msg-composer-attachment-bar 'get_size' which calculates the size of all attachments. Then, make the new code use those (fixing leaks). (personally i dont see why the list of attachments/etc is 'private' anyway, it seems to come from earlier days in evolution development where everything had to be abstracted behing a million accessors for no apparent reason other than thats what someone learnt in a 2nd year uni course was the right thing to do, under the grand assumption that every object was going to be re-usable everwhere else, which isn't how things turned out in the end). _______________________________________________ evolution-hackers maillist - [email protected] http://lists.ximian.com/mailman/listinfo/evolution-hackers
