[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Pki-devel] [PATCH] 630 Fixed PKCS12Export output.

On 7/14/2015 8:20 PM, John Magne wrote:
Reviewed for the purposes intended, refactoring and error handling. Looks good.

If tested to of course work, ACK

Thanks. Pushed to master.

Just a minor question.

It looks like there are a bunch of methods changed from private to public.
I assume this is because we are encapsulating the whole thing into a class.

Yes, since PKCS12Export will work as a library it needs to have a public interface. The public methods are setters & getters, generatePKCS12Data(), initDatabase(), and exportData(). These are the methods that the user will call as shown in main().

Also there are a couple of methods that don't have public or private, etc type declared.

Without the modifiers these methods now become package-private.
So they are still kind of private without being too restrictive. With this modifier these methods can be now be reused by other class or overridden by a subclass in the same package. I suppose in the future these methods will eventually be refactored into a PKCS12 class and reused by PKCS12Export and PKCS12Import, and then we move these classes into a separate pkcs12 package.

Anyway, it might be decent to take a quick look to make sure there are no methods that
should actually be private and of no concern to the outside world.

If so, just fix and submit.

I think the package-private is fine since we're only storing directory/file paths in the class. These methods do not modify/expose the internal data.

Endi S. Dewata

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]