code review

[ghost]

Reviewing the source code I've noticed that the Poll class is abstract and then you have two implementations UnbroadcastedPoll & BroadcastedPoll

nem-voting/src/poll.ts

Lines 79 to 133 in 616ecd1

	/**
	* Broadcasts an unbroadcasted poll and returns the resulting broadcasted poll object (as a promise)
	* @param account - NEM Account that will broadcast the poll
	* @param pollIndex - optionally provide the poll index to send the poll to.
	* If not specified the default public index is used
	* @return Promise<BroadcastedPoll>
	*/
	private broadcastPromise = async (account: Account, pollIndex?: PollIndex): Promise<BroadcastedPoll> => {
	try {
	const pollAddress = generatePollAddress(this.data.formData.title, account.publicKey);
	const link: IAddressLink = {};
	const simplifiedLink: {[key: string]: string} = {};
	this.data.options.forEach((option) => {
	const addr = deriveOptionAddress(pollAddress, option);
	link[option] = addr;
	simplifiedLink[option] = addr.plain();
	});
	const formDataMessage = "formData:" + JSON.stringify(this.data.formData);
	const descriptionMessage = "description:" + this.data.description;
	const optionsObject = {strings: this.data.options, link: (simplifiedLink)};
	const optionsMessage = "options:" + JSON.stringify(optionsObject);
	const formDataPromise = sendMessage(account, formDataMessage, pollAddress).first().toPromise();
	const descriptionPromise = sendMessage(account, descriptionMessage, pollAddress).first().toPromise();
	const optionsPromise = sendMessage(account, optionsMessage, pollAddress).first().toPromise();
	const messagePromises = [formDataPromise, descriptionPromise, optionsPromise];
	if (this.data.formData.type === PollConstants.WHITELIST_POLL) {
	const whitelistMessage = "whitelist:" + JSON.stringify(this.data.whitelist!.map((a) => a.plain()));
	const whitelistPromise = sendMessage(account, whitelistMessage, pollAddress).first().toPromise();
	messagePromises.push(whitelistPromise);
	}
	await Promise.all(messagePromises);
	const header: {[key: string]: any} = {
	title: this.data.formData.title,
	type: this.data.formData.type,
	doe: this.data.formData.doe,
	address: pollAddress.plain(),
	};
	if (this.data.formData.type === PollConstants.WHITELIST_POLL) {
	header.whitelist = this.data.whitelist!.map((a) => a.plain());
	}
	const headerMessage = "poll:" + JSON.stringify(header);
	let pollIndexAddress: Address;
	if (pollIndex) {
	pollIndexAddress = pollIndex.address;
	} else {
	pollIndexAddress = (NEMLibrary.getNetworkType() === NetworkTypes.MAIN_NET) ?
	new Address(PollConstants.MAINNET_POLL_INDEX) : new Address(PollConstants.TESTNET_POLL_INDEX);
	}
	await sendMessage(account, headerMessage, pollIndexAddress).first().toPromise();
	return new BroadcastedPoll(this.data.formData, this.data.description, this.data.options, pollAddress, link, this.data.whitelist);
	} catch (err) {
	throw err;
	}
	}

Most of methods have a lot of side effects, a domain model shouldn't talk with the network, because you are coupling the model with the infrastructure implementation, making it harder to test and change later.

The pattern you have applied is the Active Record Pattern, but due how nem-library is designed, it doesn't fit well, since we use the Repository Pattern.

On the other side, the method validate seems to contain all the logic to know if it's valid or not, right? Seems like the method with the responsibility of anouncing the transaction doesn't called it before announcing, is it responsibility of the developer to call the validate method before announcing? IMO, the Poll constructor should check the integrity of the data, and in case of be invalid, throw an exception.

nem-voting/src/poll.ts

Line 296 in 616ecd1

public validate = (): boolean => {

Reviewing the vote method, I have seen that you ask the Account instance, because it contains the private key, it could be a risk for the user. Reason? It is not in your case, but in the future, people will start creating libraries for NEM, and it will attract developers with bad intentions. We aim to not accept libraries that asks for the Account class for security reasons.

Sign a transaction and announce it should be done always outside the library, since it is not its responsibility.

nem-voting/src/poll.ts

Line 319 in 616ecd1

public vote = (account: Account, option: string): Observable<NemAnnounceResult> => {

[shierve]

I will look into the first issue.
The issue with validation is that when implementing this library I made some changes to how a poll is generated, so old polls will not be validated properly with this function. I am waiting for NanoWallet to release its latest version which is using this library, and then I will apply this automatically, but only for polls that are created after a certain block.
About asking for an account, what do you propose? I don't know how to do this without asking for the account.