nsDirPrefs should either be implemented as a singleton or a XPCOM service class so that it provides a centralized place to access directory preferences. In this case the functions of nsDirPrefs should be re-enterant. Also it has been copied from 4x and does not effectively use Mozilla constructs (specially for strings) using which can make the code cleaner and safer. Further it can be enhanced to implement an Observer for the directory prefs and notification mechanism for its clients on changes to the related prefs.

assigning to myself for now.

here's a conversation about bug #126134 and DIR_Server and reading prefs directly. cc srilatha, who this will also affect: to summarize: until we have a thin, scriptable managing service for addressbook attribute management, (like DIR_Server), we should write code that reads prefs directly. The thin service will just be a thin veneer on top of prefs, but it will keep the details of how addressbook attributes are persisted (in our case, with prefs) away from the calling code. examples: the code to delete a server will just use the pref branch code to enumerate and clear all prefs that being with a certain prefix. we'll need some way to get / set attributes using attribute name, and a key (in our case, key is a pref prefix) I'm sure we'll also need some observer / listener type setup to notify those observer who need to know when servers are added or deleted. dmose2: "so that when we delete servers, all the necessary prefs are delete. It also provides an good place to put the code that generates a unique .mab file name dmose2: neither of those things require a level of abstraction on top of prefs at all saspitzer: yes, they do. saspitzer: the first one does, because saspitzer: well, you could use prefbranch dmose2: right saspitzer: so something needs to be told "delete this server with key" dmose2: sure saspitzer: but that's all implementation on top of prefs. dmose2: but there's no reason to make every prefs getter and setter go through that code dmose2: because it's orthogonal saspitzer: you're going further and saying: saspitzer: "we don't even need a server object, we can just use prefs" dmose2: i'm saying "we don't need a server object when getting and setting prefs" dmose2: having an object to do things like generate a file name, and delete stuff is ok dmose2: but inserting that object into the middle of getting and setting prefs doesn't gain anything that i can see saspitzer: how about this: saspitzer: the new, scriptable service that rdayal (or you, or me or srilatha...) creates will allow for this: saspitzer: given a key, and an "attribute", get me the value. saspitzer: the impl for that, will be a call right into prefs. saspitzer: I don't want the caller to have to know how we store the values, I'd like to keep it all abstracted. saspitzer: that won't require an object in the middle dmose2: i'm confused saspitzer: but all callers will go through this managing service. saspitzer: like this: dmose2: wait dmose2: take a step ack dmose2: back saspitzer: ok dmose2: if i understand you correctly... dmose2: the problem you're trying to solve is the fact that the prefs have a typing system? saspitzer: no. here's what I'm getting at. dmose2: ie what details are you trying to abstract away? saspitzer: that we use prefs to implement a persistent way to remember details about addressbooks. saspitzer: prefs is just a convient way to do that. dmose2: why is abstracting away prefs worth doing? dmose2: what does it buy us? saspitzer: one sec, getting to that... saspitzer: I don't mind if the managing service (for create new ab, delete ab, get ab attributes, set ab attributes...) saspitzer: uses prefs, but here's why I want this service. saspitzer: it can also use other things, like talk to the windows registry to find out stuff, right? dmose2: i guess saspitzer: isn't that what the sun guys are hoping to do? make our code aware of outlook settings? dmose2: i wasn't aware that they were trying to do that saspitzer: I don't think a thin veneer on top of prefs is a bad thing, as it seperates the implementation (use prefs or what ever for persistent storage) from the architecture (give me a clean way from C++ and JS manage and get / set ab attributes) saspitzer: what do you think? dmose2: ok, i'm kinda ok with that dmose2: but dmose2: isn't that the whole point of contractids? dmose2: ie shouldn't someone who wants to do what you're suggesting dmose2: just implement another class that speaks dmose2: @mozilla.org/preferences;1 dmose2: or whatever it's called? saspitzer: I see your point. I guess I'm hoping that a thin veneer will leave the door open, even though I don't have much in mind for what will go there. saspitzer: clearly, DIR_Server, being heavy weight and not scriptable is not what we need. saspitzer: to work around the non-scriptable-ness saspitzer: srilatha has written a lot of js to read / write prefs directly saspitzer: duplicated the C++, basically dmose2: yeah, ok dmose2: well, so here's another thing dmose2: i don't have any big objection to the thin veneer idea saspitzer: having a managing service would allow us to clean that up. saspitzer: ok dmose2: but dmose2: i'd argue that making this code take the performance hit for DIR_Server now is the wrong thing dmose2: either way we code it, there has to be a cleanup pass later dmose2: do the efficient thing now (raw prefs reads) dmose2: and insert the veneer later saspitzer: so you'd say: "read prefs now, fix it to use the service later" dmose2: yeah dmose2: :-) saspitzer: ok, I'm cool with that. saspitzer: I doubt there is really much of a performance impact, but I'm still ok with writing less new code, rather than more. saspitzer: let me paste this entire conversation in the rdayal bug, ok with you? dmose2: given how much mozilla gets knocked for performance, i'm uncomfortable taking steps in the wrong direction dmose2: ok saspitzer: you've convinced me: if I have to leave in (reading, not writing) code, might as well use prefs directly and fix it when we fix the rdayal bug about implementing a scriptable managing service. saspitzer: we both know that the code I check in is going to live for a while, might as well make it fast and less memory consuming. dmose2: agreed; thanks