Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Smart Pointers support (see enhancement #194) #197

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

baldas
Copy link
Contributor

@baldas baldas commented Nov 24, 2014

Basic smart pointers implementation.

I also added a version of NQueens using the smart pointer support and a basic unit test (which I am not totally sure if it is done correctly).

int remsize = delegate::call(source, [](Board &b) { return b.Size(); });

/* create a new board with an extra column to place the next queen */
Board *local = new Board(remsize+1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My preference would be to have clone actually make an exact duplicate, and then have a method that would let you add a column to the end of a Board. Seems cleaner.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For instance, insertQueen could just resize the underlying array when needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah! I had that at some point, but the optimiser in me took over =P

I can restore that version tho.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, seeing as this meant to be the maximally-readable version, I'd like to keep the semantics of clone as clean as possible.

@bholt
Copy link
Member

bholt commented Nov 24, 2014

Two top-level things:

  1. What should we actually call the smart pointer class? Should be intuitive that it's an additional layer on top of GlobalAddress<T>. SharedGlobal<T>? /cc @nelsonje,@bmyerz
  2. We're not terribly consistent about naming conventions, but we often try to make our classes upper case, and our methods camelCase (with initial lower case) (and functions lower case with underscores). Could you change your methods to at least be lower-case initially?

. using #pragma once
. using DVLOG(2)
. changed the names of methods/functions to be more Grappa-like
. More readable version of nqueens-smt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants