Converting entire table to JSON data. Is there a better way? Building a 'card' database: I'm simply learning to take input data and store to database. Incorporating JSON, PDO, SQL, and enforcing my general coding skills through PHP, hopefully.

$query = $dbh->prepare("SELECT * FROM " . $table);
$query->execute();

$result = $query->fetchAll(PDO::FETCH_CLASS);

$temp = array();
$data = array();

foreach($result as $key => $val){
    foreach($val as $key2 => $val2){
        $temp[$key2] = $val2;
    }
    array_push($data, $temp);
}

echo json_encode($data);

Is there a better way to do what I am attempting?

Yes, there sure is. Effectively, what you seem to be doing can be reduced to this:

$stmt = $dbh->query("SELECT * FROM " . $table);
$data = $stmt->fetchAll(PDO::FETCH_ASSOC);
echo json_encode($data);

I'll explain how I got to those three lines above, simplifying your code step by step.
The inner loop just does not make sense:

foreach($val as $key2 => $val2){
    $temp[$key2] = $val2;
}
array_push($data, $temp);

What this does, is iterate over $val, which is either an iterable object, or an array. When doing so, you're creating a new array $temp, which is, essentially an array-version of $val. Of course, $temp is never "cleared", so the second time the inner loop is performed, $temp will be assigned new keys and values.
In short, suppose $temp looks like this, after the first iteration:

array('foo' => 123);

Then $data will look like this:

array(array('foo' => 123));

The second time, $temp might look like this:

array('bar' => 1234, 'foo' => 123);

Or, if $val had a key/property called foo, then $temp['foo'] will simply be reassigned to the new value, and $temp will look like

array('foo' => 'new value');

Either way, $data will look like:

array(array('foo' => 123), array('foo' => 'new value'));

But why the loop? Why don't you simply cast the $val to an array, and push it into the $data array?

foreach($result as $val)
{//you're not using the $key anywhere, so you can omit it
    array_push($data, (array) $val);
}

That's a hell of a lot shorter, isn't it? But you can simplify this even more. You're fetching the result as objects, only to convert it to an array. Why don't you just fetch the results as an array from the off? Simply change:

$result = $query->fetchAll(PDO::FETCH_CLASS);

To this:

$data = $query->fetchAll(PDO::FETCH_ASSOC);

You won't need the loops at all after this.
Just as an asside, fetching the results of a query is often done using a while loop. I'll save you the inner-workings-details, but last time I checked, a while loop was actually marginally more performant than a fetchAll call... The only way to be sure is to benchmark both versions, but I tend to prefer the following:

$data = array();
while($row = $query->fetch(PDO::FETCH_ASSOC))
{
    $data[] = $row;
}

Ah well, the result is the same, anyway.

Other thoughts:
There are a couple of things you might want to consider, too:

$query = $dbh->prepare("SELECT * FROM " . $table);

Is not the safest way to go about your business: What is the value of $table? If it's a value that comes from the client-side, who's to say it doesn't contain an invalid table name? something like another_db.secret_table would make for the following query:

SELECT * FROM another_db.secret_table

Fetching data that possibly wasn't intended for the client to see. That's a security issue. And a big one at that.
Next, the variable $query is not the best name. PDO::prepare returns an instance of PDOStatement, which is a prepared statement, which isn't a query anymore. That's why you often see the return value of PDO::prepare being assigned to a variable called $stmt.

Either way, with a query without a WHERE clause, prepared statements only cause more overhead. You're better of executing it from the off. Prepared statements are there when you want to insert variables into a query:

$stmt = $pdo->prepare('SELECT * FROM tbl WHERE field = :fieldVal');
$result = $stmt->execute(array(':fieldVal' => $_POST['fieldVal']));

As an added bonus, you can re-use the same prepared statement over and over, to perform the same query a couple of times:

$stmt = $pdo->prepare('SELECT * FROM tbl WHERE field = :fieldVal');
$vals = array('foo', 'bar');
foreach($vals as $val)
{
    echo 'Executing query with value: ', $val, PHP_EOL;
    $result = $stmt->execute(array(':fieldVal' => $val));
    //process $result...
}

The code above will perform the same query twice, once for the value "foo" and once with the value "bar".


Algorithm to sort a list in Python Today I've been working on this algorithm to sort a list. It works with duplicated numbers. What do you think about it? How can I improve it? Does it already exist? If it does, what is its name?

a = [1,1,5,3,2,3,4,3,99,99]  # The list I'm going to sort.
b = [None] * len(a)
x = 0
for number in a:
    for num in a:
        if number >= num:
            x += 1
    b[x-1] = number
    x = 0
b = b[::-1]
print(b)
for number in b:  #This part of the code is for the duplicated numbers.
    if number == None:
        print(b.index(number))
        b[b.index(number)] = b[b.index(number) - 1]
b = b[::-1]
print(b)

You probably already know, but just for the record, there is of course a built-in sorted function:

>>> sorted([1, 1, 5, 3, 2, 3, 4, 3, 99, 99])
[1, 1, 2, 3, 3, 3, 4, 5, 99, 99]

I'm not sure if your algorithm has a name,
but in any case I'm afraid it's not very good,
because it's very inefficient.
First of all it's quadratic: for each element in the input, it visits all other elements.

It has other inefficiencies too, for example the way it replaces the None values is also quadratic.
This is because for each value, it uses the index function of the list to find the index, and this operation scans the content of the list.

Finally, the list is reversed twice in the process, which could be avoided by a different use of indexes.

Still preserving the essence of your algorithm, consider this alternative implementation that fixes some of the above points, along with a few other improvements:

def mysort(input):
    """
    >>> mysort([1, 1, 5, 3, 2, 3, 4, 3, 99, 99])
    [1, 1, 2, 3, 3, 3, 4, 5, 99, 99]

    """
    result = [None] * len(input)
    for num in input:
        x = 0
        for other in input:
            if num <= other:
                x += 1
        result[len(input) - x] = num

    for index, num in enumerate(result):
        if num is None:
            result[index] = result[index - 1]

    return result

But ultimately, it would be better to use an algorithm that is not quadratic.
I recommend to look into insert-sort and merge-sort for exercise.


Returning interfaces in Java without significant casting I am writing some code that takes and returns interfaces following the patterns in Effective Java Third Edition. I understand the principle - we want to take in interfaces because in many cases we don't really care about the discrete implementation but rather just the features of this interface. We return interfaces for the same reason. It gives the caller the ability to use the preferred implementation class. Take this extreme contrived example:
import java.util.*;

public class Main
{
    public static List<Integer> op(List<Integer> is) {
        ArrayList<Integer> results = new ArrayList<>();

        for (Integer i : is) {
            results.add(i + 1);
        }

        return results;
    }

    public static void main(String[] args) {
        ArrayList<Integer> nums = new ArrayList(Arrays.asList(1,2,3,4));

        // This line
        ArrayList<Integer> results = (ArrayList<Integer>) op(nums);

        System.out.println(results);
    }
}

So if the caller of op wanted to use a LinkedList they could through simple casting. But to me this feels clunky. In my codebase it is littered with various casts that seem to hinder readability.
Is there a smarter way to do this? Or is this explicit casting the only way to do this properly?

Let me clarify something fist

It gives the caller the ability to use the preferred implementation class.

Even if we return interface from method, that doesn't mean that that caller can choose implementation.'

So if the caller of op wanted to use a LinkedList they could through simple casting.

If you mean something like (LinkedList<Integer>) op(nums), then this wont work and will throw java.lang.ClassCastException: java.util.ArrayList cannot be cast to java.util.LinkedList.
When method returns interface, in general, you shouldn't assume specific implementation, and thus should avoid such castings. Having interface here, for example, will give more freedom to the author of this method for future changes: imagine someone will write SuperPerfectList which works better than any other list and author decides to use it. In this case even if method changes, we won't need to refactor every place in our code where we call this method, because signature of this method hasn't changed.
If we talk about the situation where we want another implementation of this interface, then, in case of list, we can do new LinkedList(result) and get what we want.


Python tuition calculator I'm a student learning Python, I made this calculator that will tell you the tuition for the next 5 years at an increase of 3% per year. How can I improve on this and did I do anything that isn't best practice?
To revise my new version, Python tuition calculator 2.0

inState = 10000
outState = 24000
profGrad = 40000

years = []
tuitionIncrease = []

i = raw_input('Please input your type of residency, I for in-state, O for out-of-state, and G for graduate: ')

if i == 'I':
    costOfTuition = inState
elif i == 'O':
    costOfTuition = outState
elif i == 'G':
    costOfTuition = profGrad

x = 0
while x != 5:
    intMath = costOfTuition * 0.03
    tuitionIncrease.append(intMath)
    fnlMath = intMath + costOfTuition
    years.append(fnlMath)
    costOfTuition = fnlMath
    x += 1

academicYear1 = "${:,.2f}".format(years[0])
academicYear2 = "${:,.2f}".format(years[1])
academicYear3 = "${:,.2f}".format(years[2])
academicYear4 = "${:,.2f}".format(years[3])
academicYear5 = "${:,.2f}".format(years[4])

academicYear1Inc1 = "${:,.2f}".format(tuitionIncrease[0])
academicYear1Inc2 = "${:,.2f}".format(tuitionIncrease[1])
academicYear1Inc3 = "${:,.2f}".format(tuitionIncrease[2])
academicYear1Inc4 = "${:,.2f}".format(tuitionIncrease[3])
academicYear1Inc5 = "${:,.2f}".format(tuitionIncrease[4])

totalTuitionIncreaseSum = sum(tuitionIncrease)
totalTuitionIncrease = "${:,.2f}".format(totalTuitionIncreaseSum)
4
print('UNDERGRADUATE TUITION FOR THE NEXT FIVE YEARS ')
print('ACADEMIC YEAR          TUITION       INCREASE ')
print('-------------     ------------       -------- ')
print('2016-17             ' + academicYear1 + '        ' + academicYear1Inc1)
print('2017-18             ' + academicYear2 + '        ' + academicYear1Inc2)
print('2018-19             ' + academicYear3 + '        ' + academicYear1Inc3)
print('2019-20             ' + academicYear4 + '        ' + academicYear1Inc4)
print('2020-21             ' + academicYear5 + '        ' + academicYear1Inc5)
print('TOTAL TUITION INCREASE              '     + totalTuitionIncrease)

If you're currently learning Python you should probably look at Python 3
more so than Python 2.

There are only two violations for PEP8 (caught by the command-line tool,
pep8), that's very good; line 9 can be split in a multitude of ways to
go below the 80 character limit and there are more than one space before
the plus in the last line.

i = raw_input('Please input your type of residency, I for in-state, O for '
              'out-of-state, and G for graduate: ')

Also, constants should be uppercase with underscores, I'd say that
applies to inState etc.

IN_STATE = 10000
OUT_STATE = 24000
PROF_GRAD = 40000

That said, it would be easier to use a dictionary for this kind of
selection (BASE_TUITION[i]) - that might be a bit less descriptive
though.  Btw. i isn't very descriptive either.

Personally, I'd also stick with one kind of quotation marks, but YMMV.

Now for the code and other conventions.

First, code would usually be put into functions to make it reusable.  I
don't know if you're that far ahead, just to note that it would be nice
to split the input, processing and output parts into separate units.

There is no error handling for invalid input.  If I enter foo I'd
expect a (helpful) error message from the program, not a NameError as
in the current state.  For example, using the following loop, the
program will only handle valid input and print at least an indicator
that something went wrong:

costOfTuition = None
while costOfTuition is None:
    i = raw_input('Please input your type of residency, I for in-state, O for '
                  'out-of-state, and G for graduate: ')

    if i == 'I':
        costOfTuition = IN_STATE
    elif i == 'O':
        costOfTuition = OUT_STATE
    elif i == 'G':
        costOfTuition = PROF_GRAD
    else:
        print("Invalid input.")

The while loop would be more readable using for and range.  Since
x isn't actually used, _ can be used instead.

for _ in range(5):
    intMath = costOfTuition * 0.03
    tuitionIncrease.append(intMath)
    fnlMath = intMath + costOfTuition
    years.append(fnlMath)
    costOfTuition = fnlMath

The assignments to academicYear1 to 5 should be a warning sign.  If
that happens in code it's usually an indicator that it can be more
easily achieved with less variables (and less typing).

years = []
tuitionIncrease = []
academicYear = []
academicYear1Inc = []

for _ in range(5):
    intMath = costOfTuition * 0.03
    tuitionIncrease.append(intMath)
    fnlMath = intMath + costOfTuition
    years.append(fnlMath)
    costOfTuition = fnlMath
    academicYear.append("${:,.2f}".format(fnlMath))
    academicYear1Inc.append("${:,.2f}".format(intMath))

If you want it in separate loops, that's fine too.  Same goes for the
sum call at the end.  It's nicer to look at, so I kept it around.

Lastly, the formatting is mostly good with using format.  The last
print statements can be written in a nicer way though.

All in all:

IN_STATE = 10000
OUT_STATE = 24000
PROF_GRAD = 40000

costOfTuition = None
while costOfTuition is None:
    residency = raw_input('Please input your type of residency, I for '
                          'in-state, O for out-of-state, and G for graduate: ')

    if residency == 'I':
        costOfTuition = IN_STATE
    elif residency == 'O':
        costOfTuition = OUT_STATE
    elif residency == 'G':
        costOfTuition = PROF_GRAD
    else:
        print("Invalid input.")

years = []
tuitionIncrease = []
academicYear = []
academicYear1Inc = []

for _ in range(5):
    intMath = costOfTuition * 0.03
    tuitionIncrease.append(intMath)
    fnlMath = intMath + costOfTuition
    years.append(fnlMath)
    costOfTuition = fnlMath
    academicYear.append("${:,.2f}".format(fnlMath))
    academicYear1Inc.append("${:,.2f}".format(intMath))

totalTuitionIncreaseSum = sum(tuitionIncrease)
totalTuitionIncrease = "${:,.2f}".format(totalTuitionIncreaseSum)

print('UNDERGRADUATE TUITION FOR THE NEXT FIVE YEARS ')
print('ACADEMIC YEAR          TUITION       INCREASE ')
print('-------------     ------------       -------- ')

for i, year in enumerate(range(16, 21)):
    print('{}-{}             {}        {}'.format(year + 2000, year + 1,
                                                  academicYear[i],
                                                  academicYear1Inc[i]))

print('TOTAL TUITION INCREASE              ' + totalTuitionIncrease)


Manually resuming a thread I have inherited a C++ Win32 thread class which roughly works like this. I am looking for ways to improve the design; specifically I am looking for a way to get rid of the spurious Resume() call at the end of initialization. Is there a way this can be automated or omitted?

Base class

class CThread {
public:
    CThread(DWORD dwCreationFlags = 0)
        : m_hThread(Create(ThreadProc,
                           this,
                           dwCreationFlags,
                           NULL,
                           0,
                           &m_dwThreadId))
    {}

    DWORD Resume() { return ::ResumeThread(m_hThread); }

protected:
    HANDLE  m_hThread;
    DWORD   m_dwThreadId;

private:
    static HANDLE Create(LPTHREAD_START_ROUTINE pThreadProc,
                         LPVOID pParam = NULL,
                         DWORD dwCreationFlags = 0,
                         LPSECURITY_ATTRIBUTES pSecurityAttr = NULL,
                         DWORD dwStackSize = 0,
                         DWORD *pdwThreadId = NULL)
    {
        /* start thread using _beginthreadex */
        return hThread;
    }

    static DWORD WINAPI ThreadProc(LPVOID p)
    {
        CThread* pThreadObj = reinterpret_cast<CThread *>(p);
        return pThreadObj->Execute();
    }
};

So now when I make a subclass to start the thread, I need to start it suspended because otherwise it might run without being fully initialized, and then resume it after initialization:

Actual thread implementation class

class CCreateRestorePointThread
        : public CThread
{
    CCreateRestorePointThread(std::wstring const&    name,
                              zRestorePointEventType eventType,
                              zRestorePointType      restorePointType)
/* I think I need to start suspended this so that initialization can take place */
        : CThread(CREATE_SUSPENDED)
        , m_hSrClient(::LoadLibraryW(L"srclient.dll"))
        , m_bRet(FALSE)
        , m_isFinished(false)
    {
        /* Initialize Restore Point System, structures etc. */
        this->Resume(); /* I wanna get rid of this line */
    }

private:
    DWORD Execute()
    {
        /* Create Restore Point using class members */
        return m_stateMgrStatus.nStatus;
    }
};

Big Bad Issue:
The callback function for threads libraries are usually a C-function pointer. As such the thread library depends on the callback having a C ABI (not a C++ ABI). This means the function you pass into the threading API must be declared:
extern "C" void* XXXX(void*); // Assuming return and take void* parameters.
                              // Which is basically all you can do with a C API.

Note the extern "C" must be there for it to be a valid C++ program.
You are using a static member function as your callback.
static DWORD WINAPI ThreadProc(LPVOID p)

It may look like it is working but this is just pure coincidence that the ABI for static member function happens to be the same on your platform as the ABI for C functions (there is no requirement in the standard). Nor do any OS that I know give guarantees that they will not change this ABI.
You should declare it like this:
// I also suspect the you are using windows
// They also have some special macro magic you may want to look at to force
// specific calling conventions.:
//
// __cdecl    =>  One of those magic double underscore reserved identifiers.
// http://msdn.microsoft.com/en-us/library/kdzttdcb.aspx
//
extern "C" DWORD WINAPI ThreadProc(LPVOID p)
{
    CThread* pThreadObj = reinterpret_cast<CThread *>(p);
    return pThreadObj->Execute();
}

class CCreateRestorePointThread: public CThread
{
    friend DWORD WINAPI ThreadProc(LPVOID p);
    ...
};


CakePHP return url validator I have the following snippet of code that redirects a user to a specific page on successful login to the application.

if(isset($this->params['url']['next'])) {
    $url = parse_url(urldecode($this->params['url']['next']), PHP_URL_HOST);
    if( $_SERVER['SERVER_NAME'] != $url || $url == false ) {
        $pathtoredirect = $this->Auth->redirect();
    } else {
        $pathtoredirect = urldecode($this->params['url']['next']);
    }
} else {
    $pathtoredirect = $this->Auth->redirect();
}
$this->redirect($pathtoredirect);

The idea is that if a query string named 'next' exists and has a valid hostname that matches the actual sever hostname then send the user to that location otherwise send them to the default location. It should also handle really bad formatted urls by the parse_url method returning false.

Can anyone see any issues with the above code? Or offer improvements?

Thanks

EDIT

The next parameter can be added manually for example on a login link, but is also created automatically on the redirect when a user tries to access a protected action.

The reason for adding the FULL_BASE_URL is because the session also contains the app url, so if your app is on a subdirectory e.g. domain.com/myapp/ and you try and access domain.com/myapp/protectedlocation you will get redirected to the login form with domain.com/myapp/login?next=/myapp/protectedlocation which will then send you off to: domain.com/myapp/myapp/protectedlocation adding /myapp twice! Doing the full url instead STOPS this from ever happening and doing the parse url prevents a malicious user from creating a login link with their own url as the return.

public function redirect($url, $status = null, $exit = true)
    {
        if($this->Session->read('Auth.redirect'))
        {
            $url.= '?next=' . urlencode(FULL_BASE_URL.$this->Session->read('Auth.redirect'));
        }
        $this->Session->delete('Auth.redirect');
        return parent::redirect($url, $status, $exit);
    }

There is no need to urldecode() the url parameters, Cake does this for you. In fact,  you are decoding twice, which is usually a bug.

$url actually contains the hostname. That is very confusing and you should rename that.

Also, that is really deep nesting for such a simple piece of code and a lot of code duplication.

I implemented pretty much the same functionality once.

If you only want to redirect to internal resources, maybe an option would be to only allow relative urls (e. g. http://example.com/login?next=/dashboard). That would save you a lot of trouble:

$defaultRedirect = $this->Auth->redirect();

if(empty($this->params['url']['next'])) {
    return $this->redirect($defaultRedirect);
}

$path = $this->params['url']['next'];

if (strpos($path, '/') === 0) {
    return $this->redirect($path);
}

$this->redirect($defaultRedirect);

This also works around the inconsistencies of $_SERVER['SERVER_NAME'].

Why are you expecting weird urls?

Consider matching the path to your defined routes if you want to redirect to actions of your application only.


Will this test to check if an element has an attribute work? Here is my code:

hasAttr = new function(tag, attrName) {
    return (tag.attrName)
}

Will this code work? I can't test it now because I cannot acces jsfiddle and jsbin but Down For Everyone or Just Me? says something else.

jsFiddle Down For Everyone or Just Me
jsBin Down For Everyone or Just Me

Example usage:

JavaScript:

alert(hasAttr("div", "id"))

HTML:

Expected:

(In alert box)
    true
                OK

Can someone test it for me?

If it doesn't work, how should I code it?

I want hasAttr to return true or false.

I will be open to code optimization, cleaner code etc.

Thanks in advance!

To check if an HTML element has an attribute, you should use hasAttribute. If you have to support IE6 and IE7, check its existence using [name].

Given that, the implementation of hasAttr would be something like this:

hasAttr = function(tag, attrName) {
    return tag.hasAttribute ? tag.hasAttribute(attrName)
                            : tag[attrName] !== undefined;
}


Slow code pandas 8 million rows I have the following code which takes 10-15 minutes to execute. That is way too slow considering that my database is growing daily. Is there any chance to make it faster?

# Replace all empty lists ([], '[]') in dataframe with NaN's
df = df.mask(df.applymap(str).eq('[]'))

# Replace all zeros in dataframe with NaN's
df[df == 0.0] = np.nan

# Replace empty strings in dataframe with NaN's
df.replace('', np.nan, inplace=True)

# Replace all strings with value 'null' in a dataframe with NaN
df.replace('null', np.NaN, inplace=True)

Not a lot to review there;

The code is well documented and readable, the only thing I frowned at was df[df == 0.0], but my Python is probably just not good enough.

In can only think of the fact that replace can take a list of strings for to_replace, so you can merge the last 2 statements, which will give a speed up.

From a design perspective, all the routines writing to your database should never write empty lists, zeros, empty strings or nulls, but NaN instead. Then you would never have to run this script in the first place.


Printing out vector char by char I have learned a bit of C in university and I'm now learning C++ independently. I'm trying to print out std::vector<std::string> char by char. I know, that in C, strings are arrays and are terminated by '\0' terminator. Can I assume the same thing in c++? I use C++14.

This works, but I'm unsure if it's the correct way to do this. I know, I can use getline(std::cin, line) to read from stream as well. Which one is more preferred? How would you make my code better?

using namespace std;

int main()
{
    cout << "Enter text\n";

    // Get input
    vector<string> str;
    for (string temp; cin >> temp; ) {
        str.push_back(temp);
        // If input ends with enter, break loop
        if (cin.peek() == '\n') break;
    }
    cout << "\n\n";

    // Output vector string by string
    vector<string>::iterator it;
    for (it = str.begin(); it != str.end(); ++it) {
        cout << *it << '\n';
    }
    cout << "\n\n";

    // Output vector char by char
    it = str.begin();
    while (it != str.end()) {
        /* Is this (*it)[i] != '\0' correct? Are strings '\0' terminated in
         * c++ as well, and are they in vector<string> '\0 terminated? */
        for (size_t i = 0; (*it)[i] != '\0' ; ++i) {
            cout << (*it)[i];
        }
    cout << ' ';
    ++it;
    }
    cout << "\n\n";
}

So the first thing you can do is to use container loops

for (std::string &mystring : str) {
// do someting
}

The second thing would be to have a look at rbegin() and rend, which give you reverse_iterators.

You can find a very good reference here: http://www.cplusplus.com/reference/string/string/rend/

Together you would get something like that

for (std::string &mystring : str) {
    for (std::string::reverse_iterator rit=mystring.rbegin(); rit!=mystring.rend(); ++rit) {
        std::cout << *rit;
    }
}


Object destruction in Qt dialog I am new to Qt and am trying to understand object destruction and where I need to explicitly destroy objects using delete.

class MainDialog: public QWidget {
        Q_OBJECT
    public: MainDialog(QWidget *parent=0);
    private:
        QPushButton *m_newClick;
          QPushButton *m_editClick;
         MainContact *m_contact;
    }

MainDialog::MainDialog(QWidget *parent):QWidget(parent) {
    QWidget *container= new QWidget();

    QPushButton *m_newClick =new QPushButton("New", container);
    QPushButton *m_editClick= new QPushButton("Edit", container);

    QLineEdit *m_text=new QLineEdit(container);

    QVBoxLayout *vlayout=new QVBoxLayout();
    QHBoxLayout *hlayout=new QHBoxLayout();
    hlayout->addWidget(m_newClick);
    hlayout->addWidget(m_editClick);
    vlayout->addLayout(hlayout);

    connect(m_viewClick,SIGNAL(clicked()),this,SLOT(viewClick()) );
    connect(m_newClick,SIGNAL(clicked()),this,SLOT(newClick()) );

    setWindowTitle("Welcome");

    setLayout(vlayout);

}

My queries:

Will the Pushbuttons and lineEdits get destroyed once container is destroyed?
Do I need to destroy m_contact separately as it is not part of the container?

I would like a small explanation on the parent child object destruction as well.

I see a number of issues with your code, but let me answer your questions first:

1. Will the Pushbuttons and lineEdits get destroyed once 'container' is destroyed?

Yes, the pushbuttons and lineedits get destroyed when container gets destroyed

2. Do I need to destroy m_contact separately?

I do not see where m_contact is created. In the above code you just declare a pointer to a MainContact object. Nothing is created/allocated, so nothing needs to be destroyed.

Recommended reading is the detailed description of the QObject documentation:

  QObjects organize themselves in object trees. When you create a QObject with another object as parent, the object will automatically add itself to the parent's children() list. The parent takes ownership of the object; i.e., it will automatically delete its children in its destructor.

I have the impression that you do not have a clear idea what you need to do with all the pointers/objects etc. If my further comments sound like spoonfeeding, then I'm sorry for this. But I think a step-by-step discussion of your code will help you (my comments always after the code):

class MainDialog: public QWidget {

here you declare your class as a subclass of QWidget. this means your class will be a QWidget and therefore a QObject too.

        Q_OBJECT
    public: MainDialog(QWidget *parent=0);
    private:
        QPushButton *m_newClick;
          QPushButton *m_editClick;
         MainContact *m_contact;

you declare three pointers. No object will be created by this. This is just your handle to have access to the objects later.

    }

MainDialog::MainDialog(QWidget *parent):QWidget(parent) {

with QWidget(parent) you initialize the QWidget component of your class MainDialog

    QWidget *container= new QWidget();

here you create a local pointer to a QWidget and create a new QWidget object without specifying a parent of this new object. Local means that this variable is only valid within the scope of this function. To be exact: the scope is between the curly brackets {}. The variable will be destroyed when the program flow leaves the scope.

    QPushButton *m_newClick =new QPushButton("New", container);
    QPushButton *m_editClick= new QPushButton("Edit", container);

here you create two new local pointers to QPushButton objects which (!!!) hiding the pointers that you have created as private members of your class. Not all compilers will warn you about this. Your code should be just this: m_newClick =new QPushButton("New", container);

    QLineEdit *m_text=new QLineEdit(container);

as above m_text is a local pointer and will point to a newly created QLineEdit object.

    QVBoxLayout *vlayout=new QVBoxLayout();
    QHBoxLayout *hlayout=new QHBoxLayout();
    hlayout->addWidget(m_newClick);
    hlayout->addWidget(m_editClick);
    vlayout->addLayout(hlayout);

This bit is correct. You are using local pointers to the layouts which is absolutely fine; you don't need them later on. One important point here: the ownership is transferred to the layout. That means, the container object is no longer parent of the pushbuttons.

You could avoid the use of local varables by writing it like this: hlayout->addWidget(new QPushButton("New")); but then you have no handle to connect with.

    connect(m_viewClick,SIGNAL(clicked()),this,SLOT(viewClick()) );

the variable m_viewClick is not defined. This is probably just a typo here on CR. But the slot viewClick() is also not defined in the class above.

    connect(m_newClick,SIGNAL(clicked()),this,SLOT(newClick()) );

same here: newClick() is not defined.

    setWindowTitle("Welcome");

    setLayout(vlayout);

here you set the layout that contains your previously constructed objects to this MainDialog object. MainDialog becomes the new owner of vlayout and with it all the pushbuttons and lineedits.

Perhaps you also need to call show().

}

here we are at the end of the scope of MainDialog constructor function. At this point the following variables will be destroyed: container m_newClick m_editClick m_text vlayout hlayout. These are just the pointers. Since the ownership of all the objects has been transferred, no object will be destroyed. This results in a "leak" of the container and m_text object.

Summary of the problems:

the container object is useless
the three private variables m_newClick m_editClick and m_contact are not used.
the local variables m_newClick and m_editClick hide private member variables. This is a perfect example why prefixes on member variables are bad.
don't connect non-existing functions.

The following is an example with the above suggestions implemented:

class MainDialog : public QWidget
{
    Q_OBJECT
public: MainDialog(QWidget *parent=0);
public slots:
    void newClick();
    void editClick();
    void endEdit();
}

MainDialog::MainDialog(QWidget *parent)
    :   QWidget(parent)
{
    QVBoxLayout * vlayout = new QVBoxLayout;
    QHBoxLayout * hlayout = new QHBoxLayout;
    QPushButton * newButton = new QPushButton("New");
    QPushButton * editButton = new QPushButton("Edit");
    QLineEdit * lineEdit = new QLineEdit();
    hlayout->addWidget(newButton);
    hlayout->addWidget(editButton);
    vlayout->addWidget(lineEdit);
    vlayout->addLayout(hlayout);

    connect(editButton, SIGNAL(clicked()), this, SLOT(editClick()) );
    connect(newButton, SIGNAL(clicked()), this, SLOT(newClick()) );
    connect(lineEdit, SIGNAL(editingFinished(), this, SLOT(endEdit()) );

    setWindowTitle("Welcome");
    setLayout(vlayout);
    show();
}


Parsing a string of key-value pairs as a dictionary I always use nested list and dictionary comprehension for unstructured data and this is a common way I use it.

In [14]: data = """
41:n
43:n
44:n
46:n
47:n
49:n
50:n
51:n
52:n
53:n
54:n
55:cm
56:n
57:n
58:n"""
In [15]: {int(line.split(":")[0]):line.split(":")[1] for line in data.split("\n") if len(line.split(":"))==2}
Out [15]:
{41: 'n',
 43: 'n',
 44: 'n',
 46: 'n',
 47: 'n',
 49: 'n',
 50: 'n',
 51: 'n',
 52: 'n',
 53: 'n',
 54: 'n',
 55: 'cm',
 56: 'n',
 57: 'n',
 58: 'n'}

Here I am doing line.split(":")[0] three times. Is there any better way to do this?

Note it is much easier to read if you chop up the comprehension into blocks, instead of having them all on one line

You could use unpacking to remove some usages of line.split

>>> {
...    int(k): v
...    for line in data.split()
...    for k, v in (line.split(':'),)
... }
{41: 'n', 43: 'n', 44: 'n', 46: 'n', 47: 'n', 49: 'n', 50: 'n', 51: 'n', 52: 'n', 53: 'n', 54: 'n', 55: 'cm', 56: 'n', 57: 'n', 58: 'n'}

Or if the first argument can be of str type you could use dict().

This will unpack the line.split and convert them into a key, value pair for you

>>> dict(
...    line.split(':')
...    for line in data.split()
... )
{'41': 'n', '43': 'n', '44': 'n', '46': 'n', '47': 'n', '49': 'n', '50': 'n', '51': 'n', '52': 'n', '53': 'n', '54': 'n', '55': 'cm', '56': 'n', '57': 'n', '58': 'n'}


Multiple choice game This is a multiple choice game. It has a web page where you enter a question, which can have multiple answers along with multiple options.

Can anyone recommend any improvements for my code?

//correct & options are both arrays. These arrays are being passed through an AJAX call
function addQuestion($topicID, $difficulty, $isMultiple, $quest,$correct, $options){
    global $numRecords,$dbConnection,$stmt;
    connect();

    //Insert into question table & recieve the question ID for use in the answers insert stmt
     $sql = "INSERT INTO question (topic_ID,difficulty,isMultiple, question)
            VALUES($topicID,$difficulty,$isMultiple,'$quest');";
    try{
        $stmt=$dbConnection->query($sql);
            $qID = $dbConnection->lastInsertId();

        if(!$stmt)
            die("error1".$dbConnection->errorInfo());
        $stmt = "";

        for($i = 0; $i < sizeof($correct); $i++):
            $stmt .= "INSERT INTO answer (question_ID, data, isCorrect)
                VALUES($qID,'".$correct[$i]."',1);";
        endfor;

        for($z = 0; $z < sizeof($options); $z++):
            $stmt .= "INSERT INTO answer (question_ID, data, isCorrect)
                VALUES($qID,'".$options[$z]."',0);";
        endfor;
        $dd = $dbConnection->query($stmt);

        if(!$dbConnection)
            die("error2".$dbConnection->errorInfo());

    }catch (Exception $e){
        //Rollback if fails
        $dbConnection->rollback();
        return "Error rolling back: ". $e;
    }
}

Your code is vulnerable to sql-injection.  For example, if $quest contains a ' character, your INSERT will break.


Finding b and e such that b to the power of e is closest to a given number The question is to find a number with absolute minimum difference from a number of the form be. I came up with the below code which takes 4 seconds to solve each test case on an average however it has to be solved within 2 seconds.

Sample Input:

  11
  25

Sample Output:

  20

Explanation:

11 is the nearest to 32. Thus 11 - 9 = 2

25 is 52 thus there is no difference.

My code:

public static void main(String[] args) {
    Scanner in = new Scanner(System.in);
    int T = in.nextInt();    //Number of testcases

    for(int i = 0; i < T; i++){

            int number = in.nextInt();

            int sqrtNumber = (int)Math.sqrt(number);
            int powerNumber;

            int minDiff = Integer.MAX_VALUE;
            for (int b = 2; b <= sqrtNumber; b++) {
                for (int e = 1; (powerNumber = (int) Math.pow(b, e)) <= number; e++) {
                    if (number - powerNumber < minDiff)
                        minDiff = number - powerNumber;
                }
            }
            System.out.println(minDiff);

    }
}

A big performance improvement can be made when you realize you don't need to loop over all the possible exponent e. The code calculates, for each possible e, the value of be, and calculating that power takes time.

The goal here is to find b and e such that be is the closest to an integer n. If we rewrite this, it also means finding b and e where \$e\log{b}\$ is closest to \$\log{n}\$. So for a given input number n, and a given base b, we know that the closest value we're looking for will have an exponent of \$e = \left\lfloor\frac{\log{n}}{\log{b}}\right\rfloor\$.

JS1 rightfully pointed out that calculating this formula can lead to rounding errors. Since we're dealing with whole numbers, we can add 0.5 before calculating the logarithm to account for that (I tested this for all numbers from 5 to 1000000). As such, for the inner loop, you can just have:

int sqrtNumber = (int) Math.sqrt(number);
double logN = Math.log(number + 0.5); // to account for rounding issues

int minDiff = Integer.MAX_VALUE;
for (int b = 2; b <= sqrtNumber; b++) {
    int e = (int) (logN / Math.log(b));
    int powerNumber = (int) Math.pow(b, e);
    if (number - powerNumber < minDiff) {
        minDiff = number - powerNumber;
    }
}

This code only makes a single power calculation, in order to know the actual difference between the power and the target number.

Here's a result of a JMH benchmark, comparing the initial code in the question and this revised code. The input number tested is 15000, 150000 and 1500000. This is very far to take 4 seconds for each case, but the revised code is 5 times faster (Windows 10, JDK 1.8.0_102 64 bits, i5 CPU @ 2.90 GHz).

Benchmark           (number)  Mode  Cnt  Score    Error  Units
StreamTest.initial     15000  avgt   30  0,023 ±  0,001  ms/op
StreamTest.initial    150000  avgt   30  0,068 ±  0,002  ms/op
StreamTest.initial   1500000  avgt   30  0,202 ±  0,002  ms/op
StreamTest.revised     15000  avgt   30  0,005 ±  0,001  ms/op
StreamTest.revised    150000  avgt   30  0,014 ±  0,001  ms/op
StreamTest.revised   1500000  avgt   30  0,041 ±  0,001  ms/op

Code of benchmark for completeness:

@Warmup(iterations = 10, time = 700, timeUnit = TimeUnit.MILLISECONDS)
@Measurement(iterations = 10, time = 700, timeUnit = TimeUnit.MILLISECONDS)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.MILLISECONDS)
@Fork(3)
public class StreamTest {

    @State(Scope.Benchmark)
    public static class NumberContainer {

        @Param({ "15000", "150000", "1500000" })
        private int number;

    }

    static int initialCode(int number) {
        int sqrtNumber = (int)Math.sqrt(number);
        int powerNumber;
        int minDiff = Integer.MAX_VALUE;
        for (int b = 2; b <= sqrtNumber; b++) {
            for (int e = 1; (powerNumber = (int) Math.pow(b, e)) <= number; e++) {
                if (number - powerNumber < minDiff)
                    minDiff = number - powerNumber;
            }
        }
        return minDiff;
    }

    static int revisedCode(int number) {
        int sqrtNumber = (int) Math.sqrt(number);
        double logN = Math.log(number + 0.5);
        int minDiff = Integer.MAX_VALUE;
        for (int b = 2; b <= sqrtNumber; b++) {
            int e = (int) (logN / Math.log(b));
            int powerNumber = (int) Math.pow(b, e);
            if (number - powerNumber < minDiff) {
                minDiff = number - powerNumber;
            }
        }
        return minDiff;
    }

    @Benchmark
    public int initial(NumberContainer c) {
        return initialCode(c.number);
    }

    @Benchmark
    public int revised(NumberContainer c) {
        return revisedCode(c.number);
    }

}


What is good practise, more readable? I was wondering what is the better way of writing? Having several variables to show the steps or rather do method chaining?

function count(string) {
    const array = string.split("");
    return array.filter(element => element === "a").length
}

function count(string) {
    return string
        .split("")
        .filter(element => element === "a")
        .length
}

Thank you in advance.

In this case I would clearly go with the more concise version 2.

In general it can improve readability to assign complex expressions to variables with meaningful names and I would always prefer this to adding a comment. However, in this case there is neither the necessity to add a comment nor is the variable name array very meaningful. Just the opposite: You have to decode the second line by looking up array one line above if you want to understand what's going on.

Personally I would shorten the code some more:

function count(string) {
    return string.split("").filter(e => e === "a").length
}

In my opinion, the name element doesn't add much information here, so you might as well write e or x. When variable scope is very short, these short names are fine. When you read this code, you will never wonder where the hell this variable e is coming from and what it means. It literally refers to something defined on the same line.


Assign Specific Value If BitField Is True My colleague created this SQL to handle certain user selections which are based on an on/off bit flag. The goal is to identify those individual bit flags with a specific integer which will be used in a later SQL to group specific types which the flags relate to.

What would be a more elegant way of doing this on SQL-Server 2012 which would remove the proceeding if statements?

DECLARE @ct1 int;
DECLARE @ct2 int;
DECLARE @ct3 int;
DECLARE @ct4 int;

SELECT  @ct1 = ISNULL(TypeDaily, 0) ,
        @ct2 = ISNULL(TypeTerm, 0) ,
        @ct3 = ISNULL(TypePerformance, 0) ,
        @ct4 = ISNULL(TypeWeather, 0)
FROM    ReportTable
WHERE   ReportSelectionId = @ReportSelectionId;

if (@ct2 > 0)
   select @ct2 = 2

if (@ct3 > 0)
   select @ct3 = 3

if (@ct4 > 0)
   select @ct4 = 4

The Type.... fields above are valid bit columns on the ReportTable table.

This looks like a good candidate for a 'case' selection statement:

SELECT  @ct1 = case when TypeDaily = 1 then 1 else 0 end,
        @ct2 = case when TypeTerm = 1 then 2 else 0 end,
        @ct3 = case when TypePerformance = 1 then 3 else 0 end,
        @ct4 = case when TypeWeather = 1 then 4 else 0 end,
        .....
FROM    ReportTable
WHERE   ReportSelectionId = @ReportSelectionId;

Note, you seem to have an extra , at the end of your selection list.

Using the above system there is no need for the if-conditions.

An alternate syntax which reduces the process to arithmetic, would be:

SELECT  @ct1 = 1 * isNull(TypeDaily,0),
        @ct2 = 2 * isNull(TypeTerm, 0),
        @ct3 = 3 * isNull(TypePerformance,0),
        @ct4 = 4 * isNull(TypeWeather,0),
        .....
FROM    ReportTable
WHERE   ReportSelectionId = @ReportSelectionId;

In the past (before the case statement was available) I used to do calculations like the above, but depending on the circumstances, it can be less readable than the case. In this instance, I think the case is still better.


Designing a toy dependency injection library Just for learning purpose I thought to write a simple and useful dependency injection service which would just do dependency injection but also it should expose its core too which would let the client code to do some advance stuffs.

I took inspiration from here but I think it does too much apart from that I think that the Factory and Provider thing can be done via calling resolve method in my library.

I need to know if I am missing something or can I improve my code to handle more cases which I don't know yet.

const RE = /^function\s*[^\(]*\(\s*([^\)]*)\)/m;

const Color = {
  RED: 'RED',
  GREEN: 'GREEN',
  BLUE: 'BLUE',
};

class Door {
  constructor(color) {
    this.color = color;
  }

  open() {
    console.log('Kitchen door is open now!');
  }
}

class Stove {
  constructor() {
  }

  on() {
    console.log('Gas stove is turned on!');
  }

  off() {
    console.log('Gas stove is now off!');
  }
}

class TeaMaker {
  constructor(door, stove) {
    this.door  = door;
    this.stove = stove;
  }

  makeTea() {
    this.door.open();
    this.stove.on();
    console.log('Put water on stop');
    this.stove.off();
    console.log('Enjoying tea :)');
  }
}

const injector = {
  dependencies: {},
  register: (key, value) => {
    injector.dependencies[key] = value;
  },
  resolve: (deps, fn, scope={}) => {
    let args = [];
    for (let d of deps) {
      if (injector.dependencies[d]) {
        args.push(injector.dependencies[d]);
      } else {
        throw new Error(`Unkown Dep: ${d}`);
      }
    }

    return () => {
      return fn.call(scope, ...args, ...arguments);
    };
  },
  get: (fn) => {
    let deps = fn.toString().match(RE)[1].replace(/ /g, '').split(',');
    let args = [];
    for (let d of deps) {
      if (injector.dependencies[d]) {
        args.push(injector.dependencies[d]);
      } else {
        throw new Error(`Unkown Dep: ${d}`);
      }
    }
    return new fn(...args);
  }
};

injector.register('door',  new Door(Color.RED));
injector.register('stove', new Stove());

let makeTea = injector.resolve(['door', 'stove'], function(door, stove) {
  door.open();
  stove.on();
  console.log('Put water on stop');
  stove.off();
  console.log('Enjoying tea :)');
});

makeTea();
let teaMaker = injector.get(TeaMaker);
teaMaker.makeTea();

Output:

Kitchen door is open now!
Gas stove is turned on!
Put water on stop
Gas stove is now off!
Enjoying tea :)
Kitchen door is open now!
Gas stove is turned on!
Put water on stop
Gas stove is now off!
Enjoying tea :)

In your injector you're exposing the dependencies object, in a way that all could access.

You should consider to hide it instead, so people will use just the API as it should be.

This happened because you choose to define your component just with a plain object approach.

If you want to write a lib or an utility, it is better to use one of the module pattern.

For example wrapping your code in a function if you mean to use it within a browser:

!function () {
    const dependencies = {};

    const injector = {
      // my code here
    };

    return injector;
 }()

Or with the node modules if you use in node:

const dependencies = {}

const injector = {
    // your code here..
}

export default injector

So you could expose just the API function you want.

This helps the developer to better understand how you design the lib to be used.


Writing a Counter object to a CSV-file in Python 3 In Python 3.4, a Counter object called cnt like this:

Counter({'0200': 3, '3000': 2, '3011': 2, '0210': 1, '4000': 1})

is to be written to a comma-separated file.

I first tried with CSV and DictWriter that I had not used before, but I kept getting misty errors and was pressed for time, so I decided to make this simple version (it works):

outfile = infile[:-4] + '_freq.csv'
fp = open(outfile, encoding='utf-8-sig', mode='w')
fp.write('KMC|freq\n')
for tag, count in cnt.items():
    fp.write('{}|{}\n'.format(tag, count))

fp.close()

Then when I had some time I felt I should learn to use DictWriter and CSV, assuming it'd be 'better' or 'more pythonic'.

That turned into this (works too):

outfile2 = infile[:-4] + '_freq2.csv'
with open(outfile2, encoding='utf-8-sig', mode='w', newline='') as f:
                # Note: empty newline to avoid blank lines in the output file
    fieldnames = ['KMC', 'freq']
    writer = csv.DictWriter(f, fieldnames=fieldnames, delimiter='|')
    writer.writeheader()
    for tag, count in cnt.items():
        writer.writerow({'KMC': tag, 'freq': str(count)})

The trick with the empty newline to avoid blank lines is from here.

I like the first approach better. I think CSV and DictWriter are intended for much larger amounts of columns than the two small ones I got. But I'm no expert.

Any learned comments?

Simplicity

I think that, for an easy task like this, the first version is better as it is considerably simpler while accomplishing the same goal.

It can still be improved though:

Cleaner deletion of file extension

infile[:-4] is probably used to remove the extension from the file, but it is not obvious, I suggest infile.replace(".txt","")(
where the extension may be different from txt but you get the idea).

Context managing

It is so easy to forget to close a file, it is better to use with that will close it automatically every time:

with open(outfile, encoding='utf-8-sig', mode='w') as fp:
fp.write('KMC|freq\n')
for tag, count in cnt.items():
    fp.write('{}|{}\n'.format(tag, count))

Naming

cnt is a mysterious name, I have no idea what kind of data it may contain, I suggest coming up with a better name.


Creating a TCP Listener and receiving data I am very new to network programming and I'm thinking I have probably misconstrued the creation of an appropriate TCP listener. The code I have below works perfectly, but I have a feeling it's a "hack and slash" miracle and could come apart easily when set into my production environment. What can I do to improve this receive message?

Objective

The application should be able to receive a string and parse it into an xml string. Later, after a set of validation and data collection is completed, a response will be sent back to the client.

Receive Message

private static void ReceivePortMessages()
{
    int requestCount = 0;
    tcpListener.Start();
    Debug.Print(" >> Server Started");
    tcpClient = tcpListener.AcceptTcpClient();
    Debug.Print(" >> Accept connection from client");

    while (true)
    {
        try
        {
            requestCount = requestCount++;
            NetworkStream networkStream = tcpClient.GetStream();
            byte[] bytesFrom = new byte[10025];
            networkStream.Read(bytesFrom, 0, (int)tcpClient.ReceiveBufferSize);
            Stopwatch sw = new Stopwatch();
            sw.Start();
            string dataFromClient = System.Text.Encoding.ASCII.GetString(bytesFrom);
            dataFromClient = dataFromClient.Substring(0, dataFromClient.IndexOf("\0"));

            XmlDocument xm = new XmlDocument();
            xm.LoadXml(string.Format("<root>{0}</root>", dataFromClient));
            XmlElement root = xm.DocumentElement;
            string rootName = root.FirstChild.Name;
            RouteInboundXML(rootName, dataFromClient, sw);
        }
        catch (ArgumentOutOfRangeException ex)
        {
            Debug.Print("ReceivePortMessages: Remote client disconnected. " + ex.ToString());
            tcpClient.Close();
            tcpListener.Stop();
            ReceivePortMessages();
            return;
        }
        catch (Exception ex)
        {
            Debug.Print("ReceivePortMessages: " + ex.ToString());
            tcpClient.Close();
            tcpListener.Stop();
            ReceivePortMessages();
            return;
        }
        Debug.Print(" >> exit");
    }
}

Send Message

private void SendReply(string reply)
{
    try
    {
        NetworkStream networkStream = _TcpClient.GetStream();
        string serverResponse = reply;
        Byte[] sendBytes = Encoding.ASCII.GetBytes(serverResponse);
        networkStream.Write(sendBytes, 0, sendBytes.Length);
        networkStream.Flush();
        Debug.Print(" >> " + serverResponse);
    }
    catch (ArgumentOutOfRangeException ex)
    {
        Debug.Print("SendReply: Remote client disconnected. " + ex.ToString());
        _TcpClient.Close();
        _TcpListener.Stop();
        ReceivePortMessages();
        return;
    }
}

A common convention is to prefix class members with _ so it's easier to see what a local variable is and what's a class member. In your case it would be _tcpClient or even _TcpClient.

This is a static method which indicates that it's possibly part of a static class. This is almost always a bad idea. static means it's global and global state is bad because it can get you into all kinds of trouble once your program grows in complexity.

You have a StopWatch but you're not actually using it.

I assume you use the Substring because the buffer is larger than the actual data received so you find the end by looking for the first \0. GetString() provides an overload which allows you to specify offset and length so this becomes unnecessary.

MSDN stipulates that you must close the NetworkStream once you are done with it because the client will not release it. Given that streams are IDisposable it would be best wrapped in a using block.

TcpClient is IDisposable as well so should be wrapped in a using block to make sure it gets cleaned up properly.

The method is recursive and there is no way to bail out because in case of error it calls itself again. This means your call stack is growing indefinitely and will fall over at some point. Each call will also allocate a new receive buffer every time.
A while loop with a break condition would probably be the better choice.

So in total the refactored code could look like this:
private static void ReceivePortMessages()
{
    byte[] receiveBuffer = new byte[10025];
    while (!_QuitProcessing)
    {
        int requestCount = 0;
        _TcpListener.Start();
        Debug.Print(" >> Listener Started");
        using (var tcpClient = _TcpListener.AcceptTcpClient())
        {
            Debug.Print(" >> Accepted connection from client");

            using (var networkStream = tcpClient.GetStream())
            {
                while (!_QuitProcessing)
                {
                    try
                    {
                        requestCount = requestCount++;
                        var bytesRead = networkStream.Read(receiveBuffer, 0, (int)tcpClient.ReceiveBufferSize);
                        if (bytesRead == 0)
                        {
                            // Read returns 0 if the client closes the connection
                            break;
                        }

                        string dataFromClient = System.Text.Encoding.ASCII.GetString(receiveBuffer, 0, bytesRead);

                        XmlDocument xm = new XmlDocument();
                        xm.LoadXml(string.Format("<root>{0}</root>", dataFromClient));
                        XmlElement root = xm.DocumentElement;
                        string rootName = root.FirstChild.Name;
                        RouteInboundXML(rootName, dataFromClient, sw);
                    }
                    catch (Exception ex)
                    {
                        Debug.Print("ReceivePortMessages: " + ex.ToString());
                        break;
                    }
                }
            }
            Debug.Print(" >> stopped read loop");
        }
        _TcpListener.Stop();
    }
}

It's still not ideal because the Read will block unless data is there to be received or the connection is closed. So if the other side stops sending data but keeps the connection alive you still have no nice way to bail out.


Interactive multiple choice code This isn't so much, HOW do I make it work, as this code works, rather how could I make it better. The way this code works is it guides a person to the correct answer by getting "Yes", "No" or "I don't know" (in some cases from the person).

It's intended to run in HTML5/JavaScript (kind of like an online interactive guide). This code actually works, but it uses the JavaScript "prompt" for the input, and frankly, it's pretty ugly. Any suggestions on how to make it either look or work better?

First the JavaScript, as it's the main engine:

function mc1() {
var n = prompt("Do you have Medicare benefits (Y=Yes, N=No, I=I don't know)", " ");

if( n == "Y"){
   mcs2();
}
else if (n="N"){
    mcs1a();
   }

else if (n="I"){
   mcs1b();
 }

else {
alert("Enter a valid choice Y,N or I!");
mc1();
}
}

function mcs1a(){
var n1=prompt("Have you applied and been denied Medicare Coverage (Y= Yes,  N= No,  I = I don't know)", "");
if (n1=="Y"){
mc3a();
}
else if (n1=="N"){
mc2b();
}
else if (n1=="I"){
mc2b();
}
else {
alert("Enter a valid choice:  Y, N or I!");
}
}

function mcs1b(){
alert("Refer to Social Security Administration to check status, (CODE E01M1)!");
}

function mcs2(){
var n2=prompt("Do you have Medicare Part A AND B ( Y=Yes, N=No, I=I don't know)","");
if (n2=="Y"){
mcs3();
}
if (n2=="N"){
alert("Refer to Social Security to apply for the other part (Code E01M1)!");
}
if (n2=="I"){
alert("Refer to Social Security Administration to check status (Code E01M1)!");
}
else {
alert("Enter a valid choice: Y, N, or I!");
}
}

function mcs3(){
var n3=prompt("Would you like to apply for HF MEdicare (Y=Yes, N=No)","");
if (n3=="Y"){
mcs4a();
}
if (n3=="N"){
mcs4();
}
else {
alert("Enter a valid choice: Y or N!");
}
}

function mcs4(){
var n4=prompt("Do you have a caretaker relative who claims you on their taxes (Y=Yes, N=No)","");
if (n4=="Y"){
alert("Follow standard prequalification with caretaker relative as the income");
}
if (n4=="N"){
alert("Refer to the HRA/LDSS/SLDSS to apply for benefits (CODE E01H)");
}
else {
alert("Enter a valid choice: Y or N!");
}
}

function mcs4a(){
alert("Warm transfer to Medicare Advertising to apply for HealthFirst Medicare (Code WTS)");}

mc1();
<html>
<head />
<body>
<script src="testdrop.js"></script>

</body>
</html>

Bad names!!

Very poor naming, so much so that your code is almost unreadable and definitely unusable.
Use constants for variables that do not change. eg var n1= should be const n =
Always! indent your code.
Spaces between operators var n1=prompt( should be var n1 = prompt(
else on same line as closing } eg } else if (
Use === rather than == and !== rather than !=
The code is broken and throws errors because you have missing function calls. Next time you post on code review please make sure that the code works, or clearly mark code stubs.
A lot of repeated code could be put together as a function.

eg Ask question function

function askQuestion(message, yesCall, noCall, unsureCall) {
    const result = prompt(message, "").toLowerCase();
    if ( result === "y") { yesCall() }
    else if (result === "n") { noCall() }
    else if (result === "i") { unsureCall() }
    else { askQuestion(message, yesCall, noCall, unsureCall) }
}

Then you can ask the question

function mcs1() {
    askQuestion(
        "Do you have Medicare benefits (Y=Yes, N=No, I=I don't know)",
         mcs2, mcs1a, mcs1b
    );
}

User friendly

When creating interfaces you should never have ask the user to correct themselves due to improper use of the interface.

If you ask for a Y or N and you can not control the keys, don't respond with "Enter a valid choice Y,N or I!" as it is a pointless and frustrating step for the user. Just ask the question again. "Try again... Do you have Medicare benefits? Y/N"

Best is to ensure that the user can only make correct selections.

Use String.toLowerCase() or String.toUpperCase() to eliminate the need to differentiate between upper and lower case input.

Use a dialog with clickable options, or vet the key inputs with a keyboard event listener (not available on prompt)

Alert and Prompt are evil

Don't use prompts or alerts. Use the HTML5 element dialog (or use standard HTML) and create custom dialogs.

Custom dialogs  are non blocking and let you control the inputs, style, and much more. Giving a much better feel and flow to the interrogation.

See example how to use <dialog> element. (Note: full support for <dialog> element is only on Chrome for the moment. A polyfill can be found on MDN dialog reference page.)

Separate questions from code.

You have coded the questions into the source such that if you make a change to the questions you will need to make changes to the source code.

These types of apps (survey or interrogations) are generally a class of finite state machine. The state (a question or message) will always follow a previous known state, and the following state will depend on the current answer.

Seperate the state flow from the logic in such a way that you do not need to code the questions. You should be able to present any list of questions as a dataset (eg JSON) without needing to change any of the source code.

If the data structure is well defined, it also allows for non coders to create and change the interrogation.

An example.

The example may be a little advanced but have a look anyways, take what you can and feel free to ask questions in the comments.

It uses the dialog element to create the dialogs.
Async functions and promises to wait for user input.
Event listeners to get user clicks.
Defines a state machine via the object interrogation that uses named properties to connect states. The function interrogate takes a state name and use it to determine how to present and act on that state.

setTimeout(()=>interrogate("start"),0);

/* Structure of interrogation All names are case sensitive [] means optional
  name: { // state name
    [action]: string, // one of two actions "reset" or "showResult"
    [question]: string,  //  The question if included the must have Y,N and/or [I]
    [Y]: string  // name of state for yes result
    [N]: string  // name of state for no result
    [I]: string  // name of state for unsure result
    [message]: string, // Message to display. This is only optional if question is defined
    [next]: string // name of next state. If omitted then defaults to "exit"
  }
*/

const interrogation = {
    results: "",
    start: {
        action: "reset",
        question: "Do you have Medicare benefits?",
        Y: "partsAB",
        N: "deniedMedicare",
        I: "refSocialSecurityAdministration"
    },
    partsAB: {
        question: "Do you have Medicare Part A AND B",
        Y: "applyForHF",
        N: "unknown",
        I: "unknown"
    },
    applyForHF: {
        question: "Would you like to apply for HF MEdicare",
        Y: "applyForHFInfo",
        N: "haveCaretaker"
    },
    haveCaretaker: {
        question: "Do you have a caretaker relative who claims you on their taxes",
        Y: "haveCaretakerInfo",
        N: "dontHaveCaretakerInfo",
    },
    deniedMedicare: {
        question: "Have you applied and been denied Medicare Coverage",
        Y: "unknown",
        N: "unknown",
    },
    startAgain: {
        question: "Would you like to restart?",
        Y: "start",
        N: "exit",
    },
    unknown: {
        message: "Arrrghhhh. computer meltdown.. the pain, the pain... :P",
        next: "startAgain",
    },
    refSocialSecurity: {
        message: "Refer to Social Security to apply for the other part (Code E01M1)!"
    },
    refSocialSecurityAdministration: {
        message: "Refer to Social Security Administration to check status (Code E01M1)!"
    },
    haveCaretakerInfo: {
        message: "Follow standard prequalification with caretaker relative as the income."
    },
    dontHaveCaretakerInfo: {
        message: "Refer to the HRA/LDSS/SLDSS to apply for benefits (CODE E01H)",
    },
    applyForHFInfo: {
        message: "Transfer to Medicare Advertising to apply for HealthFirst Medicare"
    },
    exit: {
        action: "showResult",
        message: "Thank you for your participation.",
    },
}

async function askQuestion(message, includeNotSure) {
    return new Promise(selectionMade => {
        quesionMenuEl.addEventListener("click", event => {
            selectionMade(event.target.value);
            event.preventDefault();  // stops the dialog from permanently closing
            questionDialog.close();
        }, {once: true});
        questiomMessageEl.textContent = message;
        notSureButton.style.display = includeNotSure ? null : "none"; //null is default
        questionDialog.showModal();
    });
}
async function showMessage(message) {
    return new Promise(selectionMade => {
        messageMenuEl.addEventListener("click", event => {
            messageDialog.close();
            event.preventDefault(); // stops the dialog from permanently closing
            selectionMade("OK");
        }, {once: true});
        messageMessageEl.textContent = message;
        messageDialog.showModal();
    });
}
function interrogate(stateName = "exit") {
    const state = interrogation[stateName];
    if(state.action === "reset") { interrogation.results = "" }
    else if(state.action === "showResult") { console.log("Result: " + interrogation.results) }
    if (state.question) {
        askQuestion(state.question, state.I !== undefined)
            .then(result => {
                if (result !== undefined) {
                    interrogation.results += result;
                    interrogate(state[result]);
                } else {
                    interrogate(stateName); // clicked outside button
                }
            });
    } else if (state.message) {
        showMessage(state.message)
            .then(() => {
                if (stateName !== "exit") { interrogate(state.next) }
            });
    }
}
<dialog id="questionDialog">
  <form method="dialog">

    <menu id="quesionMenuEl">
      <button value="Y">Yes</button>
      <button value="N">No</button>
      <button value="I" id="notSureButton">Not sure</button>
    </menu>
  </form>
</dialog>
<dialog id="messageDialog">
  <form method="dialog">

    <menu id="messageMenuEl">
      <button value="ok">OK</button>
    </menu>
  </form>
</dialog>


Finding matching PDF files in different folders and merging the two files into one I have code which currently loops through one folder, finds two matching pdf-filenames and merges them into 1 pdf-file always in the same order. File 1 then File 2. My code is slow, maybe because merging PDF files takes a long time, but also because it has to search one folder and match filenames. Before I searched through two folders, but that took too long. I do not want to write the filenames to an Excel-file and then match, but I want to merge the PDF files immediately when VBA finds a match. I found some code and adjusted it to my needs and put some new things into it.

I have seen a lot of code using Acrobat or Pdftk, but I do not have Acrobat and Pdftk does not work with me even though I do exactly what is written on several sites. For some reason Pdftk does not merge my files.

My code matches filenames based on the first part of the filename, before the AnotherWord 2014.pdf or before SomeWord.pdf.

Example document name1: John Doe SomeWord.pdf
Example document name2: John Doe AnotherWord 2014.pdf

Please look at this code and please post code which will do the same thing, only much faster. My code isn't the best, but it works. I use PDF reDirect Pro v2.5.2 (freeware) and a reference to the program.

    Sub MergePDF_Files()
'Only works with PDF reDirect Pro v2.5.2
'And needs to have a reference to PDF_reDirect_v2500 and PDF reDirect Pro Remote Control
Dim fs As Object
Dim fld As Object
Dim fld2 As Object
Dim objFile As Object
Dim objFile2 As Object
Set fs = CreateObject("Scripting.FileSystemObject")
Dim oPDF As New PDF_reDirect_v25002.Batch_RC_AXD
Dim TempBool As Boolean
Dim Files_to_Merge(1) As String
Dim ObjFileName() As String
Dim CellNameValue() As String
Dim ofn As String
Dim cnv As String
Dim i As Integer

Set fld = fs.GetFolder("C:\MyFolder\PDF Merge\Merging Area")
Set fld2 = fs.GetFolder("C:\MyFolder\PDF Merge\Merging Area\Merged")
i = 1

For Each objFile In fld.files
For Each objFile2 In fld.files
    CellNameValue() = Split(objFile.Name, " SomeWord.pdf")
    cnv = CellNameValue(0)
    ObjFileName() = Split(objFile2.Name, " AnotherWord 2014.pdf")
    ofn = ObjFileName(0)
    Files_to_Merge(1) = fld & "\" & ofn & " AnotherWord 2014.pdf"
    Files_to_Merge(0) = fld & "\" & cnv & " SomeWord.pdf"
    If StrComp(ofn, cnv) = 0 Then
    With oPDF

        TempBool = .Utility_Merge_PDF_Files(fld2 & "\" & ofn & " AnotherWord 2014.pdf", Files_to_Merge) 'The file merges here unless it generates an error and goes to If Not TempBool Then...

        If Not TempBool Then
            'MsgBox "An Error Occured: etc.
        Else
            'Optional
        End If
    End With
    End If
    i = i + 1
    Next objFile2
     i = i + 1
    Next objFile
    Set oPDF = Nothing
End Sub

I cannot do full test as I do not have that PDF software of yours. But the idea is that process all file names in that folder 1, store them in a collection, also if the file name ends with " SomeWord.pdf" then add to another collection.

When done, process the other collection and see if a matched " AnotherWord 2014.pdf" found in first collection. Merge if found.

Reworked after Trial is downloaded.

Option Explicit

Private oFullList As Object, oMyList As Object, oFSO As Object

Private Const FDR1 = "C:\Test\"
Private Const FDR2 = "C:\Test\Merged\"

Private Const sAppend1 = " SomeWord.pdf"
Private Const sAppend2 = " AnotherWord 2014.pdf"

Private oPDF As New PDF_reDirect_v25002.Batch_RC_AXD
Private lFiles As Long

Sub Main()
    lFiles = 0
    Debug.Print Now & vbTab & "Program Started"
    '===[ Create Collection Objects ]===
    ' FullList stores all the filenames in the directory
    If oFullList Is Nothing Then
        Set oFullList = CreateObject("System.Collections.ArrayList") ' Create if not created
    Else
        oFullList.Clear ' Remove all old entries
    End If
    ' MyList only stores filenames that will be checked for merge
    If oMyList Is Nothing Then
        Set oMyList = CreateObject("System.Collections.ArrayList") ' Create if not created
    Else
        oMyList.Clear
    End If
    '===[ Check FileSystemObject ]===
    If oFSO Is Nothing Then
        Set oFSO = CreateObject("Scripting.FileSystemObject")
    End If
    '===[ Main procedures ]===
    CheckFiles
    'ListFiles ' Skipped, debug only
    ProcessMergeFiles
    '===[ Clean Up ]===
    Set oFullList = Nothing
    Set oMyList = Nothing
    Set oFSO = Nothing
    Set oPDF = Nothing
    '===[ Print Summary ]===
    Debug.Print Now & vbTab & "Program Finished. " & lFiles & " merged PDFs has been created"
End Sub

Private Sub CheckFiles()
    Dim oFDR As Object, oFile As Object, sTmp As String, n As Long
    '===[ Process all filenames of FDR1 ]===
    For Each oFile In oFSO.GetFolder(FDR1).Files
        sTmp = oFile.Name
        ' Add to FullList collection
        oFullList.Add sTmp
        ' Only add to MyList collection if it ends with sAppend1
        n = InStr(1, sTmp, sAppend1, vbTextCompare)
        If n > 0 Then
            oMyList.Add Left(sTmp, n - 1) ' Stores only the prefix
        End If
    Next
End Sub

Private Sub ListFiles()
    Dim i As Long
    Debug.Print "=== Listing " & oMyList.Count & " item(s) in oMyList ==="
    For i = 0 To oMyList.Count - 1
        Debug.Print i + 1 & vbTab & oMyList(i)
    Next
End Sub

Private Sub ProcessMergeFiles()
    Dim i As Long, sTmp As String
    For i = 0 To oMyList.Count - 1
        sTmp = oMyList(i)
        If oFullList.Contains(sTmp & sAppend2) Then
            'Debug.Print "Merge file pair found: """ & sTmp & """"
            MergeFiles sTmp
        End If
    Next
End Sub

Private Sub MergeFiles(sFilePrefix As String)
    Dim bSuccess As Boolean
    Dim aFiles() As String ' <- Required datatype

    If oPDF Is Nothing Then
        Set oPDF = New PDF_reDirect_v25002.Batch_RC_AXD
    End If

    ReDim aFiles(1)
    aFiles(0) = FDR1 & sFilePrefix & sAppend1
    aFiles(1) = FDR1 & sFilePrefix & sAppend2

    With oPDF
        bSuccess = .Utility_Merge_PDF_Files(FDR2 & "\" & sFilePrefix & sAppend2, aFiles)
        If bSuccess Then
            Debug.Print "Merged pdf files with prefix """ & sFilePrefix & """"
            lFiles = lFiles + 1
        Else
            Debug.Print "Failed to merge pdf files with prefix " & sFilePrefix
            Debug.Print "ERR(" & .LastErrorNumber & "):" & .LastErrorDescription
            .Reset_Errors
        End If
    End With
End Sub


Casting between types for shared_ptr I'm implementing an Entity-Component-System. Basically, an entity is a wrapper around an ID, a component is just a POD struct, and a System is a class that does work on components corresponding to the Entities subscribed to that system.

I'd like to take advantage of smart pointers, so the cleanup will be much smoother. However, I've ended up with a rather ugly looking function for getting the component attached to an Entity.

For this implementation of an ECS, all components inherit from a base struct just simply called Component. Entities have a hashmap of the components they own, with that components ID as the key.

Entity.h

//Forward declaration
struct Component;

struct Entity
{
    /*A unique ID*/
    const unsigned long long int kID;

    /*A map containing all components held by this entity*/
    std::map<int, std::shared_ptr<Component>> component_map_;
};

Component.h

typedef int COMPONENT_ID;

/*Base component class*/
struct Component
{
    static const COMPONENT_ID kTYPE = -1;
};

/*Components*/
struct CPosition : public Component
{
    static const COMPONENT_ID kTYPE = 0;
    float x, y;
};

EntityManager.h templated function for getting a component from an entity

/*Gets a component from an entity*/
template<typename T>
std::shared_ptr<T> GetComponent(std::shared_ptr<Entity> entity) {
    return std::shared_ptr<T>((T*)entity->component_map_.at(T::kTYPE).get());
}

An example of how it would be used:

int main(int argc, char** argv)
{
    /*Create the entitymanager*/
    EntityManager em;
    /*Create an entity*/
    auto a = em.CreateEntity();
    /*Create a new Position Component*/
    /*struct CPosition : public Component */
    auto component = new CPosition();
    component->x = 55;
    component->y = 200;
    /*Add the CPosition component to the entity a*/
    em.AddComponent(a, component);
    /*Get the component back from the entity a*/
    auto mcomponent = em.GetComponent<CPosition>(a);
    return 0;
}

Is there a cleaner way to get the component back from the entity, without grabbing the raw pointer and casting it and sticking it back into a shared_ptr? What about having the static ID of components and using that to map the components in the entity? Any better solutions?

template<typename T>
std::shared_ptr<T> GetComponent(std::shared_ptr<Entity> entity) {
    return std::shared_ptr<T>((T*)entity->component_map_.at(T::kTYPE).get());
}

Noooo. This is a misuse of shared_ptr's constructor (trying to create a brand-new shared_ptr to manage an object that is already managed), and will lead to double-free bugs or worse. What you meant was to cast the existing smart pointer to your desired smart pointer type:

template<typename T>
std::shared_ptr<T> GetComponent(std::shared_ptr<Entity> entity) {
    std::shared_ptr<Component> ptr = component_map_.at(T::kTYPE);
    return std::static_pointer_cast<T>(ptr);
}

This is a moderately obscure corner of smart-pointer-land, and you shouldn't be expected to know it; but now that you see it, I bet you're smacking your forehead in disgust. :)

By the way, I see no reason to provide static const kTYPE = -1 in the base class; doesn't that just counterproductively move the diagnosing of several kinds of typo-bugs from compile-time to run-time?

Also, note that std::map is not a hashmap; it's a tree-map. If you really want a hashmap (or if you don't care), and you're using C++11 or later, you should switch to std::unordered_map.

  What about having the static ID of components and using that to map the components in the entity? Any better solutions?

C++11 has a feature designed specifically for this use-case, although I personally have zero experience with it — it's spelled std::type_index. What you're looking for is a std::unordered_map<std::type_index, std::shared_ptr<Component>>.


API wrapper for Clojure I wanted to wrap the jkeymaster library in a Clojure wrapper (for my own use, but perhaps also to save others some time). I'm just learning Clojure so I'm still not quite sure what "idiomatic libraries/APIs" look like. In Ruby, I would have created an object with methods, but in one of the clojure books they recommended letting users manage state themselves, so I have function that creates a provider, and this provider must then be passed to the register function.

Here's first an example of usage:

(ns keymaster.core
  (:require [keymaster.keymaster :as km]))
(defn -main
  "I don't do a whole lot ... yet."
  [& args]
  (km/register (km/provider) "control shift 1" #(println "hi")))

And here is the full text of the "library".

(ns keymaster.keymaster
  (:gen-class)
  (:use (com.tulskiy.keymaster.common)))

(defn provider []
  "Gets and initiates a keymaster provider, which must be passed to register to register shortcuts"
  (let [provider (com.tulskiy.keymaster.common.Provider/getCurrentProvider true)]
    (.init provider)
    provider))

(defn- conv-keystroke [x]
  "Takes keystroke in the form \"control shift 1\" and returns a Keystroke class"
  (javax.swing.KeyStroke/getKeyStroke x))

(defn- conv-listener [f]
  "Takes a function with one argument, which will get passed the keycode, and creates a listener
   Todo: How to accept a function with or without a parameter to accept hotKey?"
   (proxy [com.tulskiy.keymaster.common.HotKeyListener] [] (onHotKey [hotKey] (f))))

(defn register
  [provider shortcut listener]
  "Registers a shortcut on provider, which will trigger listener (with one argument)"
  (let [k (conv-keystroke shortcut)
        l (conv-listener listener)]
    (.register provider k l)))

In addition to any general style feedback, both about the coding, and the design of the API, I'm also wondering if there is any way to have conv-listener accept either a 0 or 1-arity function. Right now I made the choice of accepting a 0-arity function, but if someone wanted to pass a 1-arity function, where the first argument would be the name of the keycode called (hotKey), which I would call with (f hotKey), I would have to rewrite... Possible to allow both? (1-arity is inconvenient for anonymous functions where we really don't need it - which I suspect is most of the time, since we can bind different hotkeys to different listeners).

As a wrapper goes I think this looks fine. The feedback I do have is that I wouldn't use let as often as you do and would use the treading macro (-> ...) instead.

For example:

(-> (com.tulskiy.keymaster.common.Provider/getCurrentProvider true)
      .init)

does the same as

(let [provider (com.tulskiy.keymaster.common.Provider/getCurrentProvider true)]
  (.init provider)
  provider)

and it's easier to read and shorter. You could also do it this way:

(.init (com.tulskiy.keymaster.common.Provider/getCurrentProvider true))


XMLHTTPrequest create own service I'm trying to create my own service, $hR.  This service uses XMLHTTPrequest to get data from a URL with promise.

;(function(global){

    var http = function(){
        var self = {}
        self.get = function (url){

            var ajax = new XMLHttpRequest();
            ajax.open('GET',url,true)
            ajax.send()
            var p = new Promise(function(resolve, reject){
                ajax.onreadystatechange = function (){
                    if(ajax.readyState == 4){
                        var obj = JSON.parse(ajax.responseText)
                        resolve(obj)
                    }
                }
            })
            return p;
           }//Get Methood
        return self;
        }
    global.$hR = http();
}(window));

It works great for me, but I created this service only to study and learn.  I want to improve my code.

Can someone give me a more expert way to this same service?

First of all, I would stop what you're doing. What you're essentially doing is what the new fetch is for. It does an asynchronous HTTP call, and returns a promise. For older browsers, there's a polyfill.

Should you choose to proceed, here's a few things:

I am not sure why http is a function that returns an object that attaches to $hR. A lot of the code is unnecessary. If what you're doing is just exposing $hR as a global and having a bunch of functions on it and maybe a private scope to put stuff in, then the following will do:

;(function(ns){               // 2

  ns.get = function(){...}    // 3

}(this.$hR = this.$hR || {}); // 1

What this code does is:

Uses an existing $hR global or create a new object and assign it as the $hR global.
The global is passed into the closure as ns. ns will represent your global object.
Attach functions to ns. In this case, the get function.

Now for your AJAX code. A cleaner way to define this code is to put everything inside the promise constructor. This way, the promise function just looks like a wrapper.

ns.get = function(url){
  return new Promise(function(resolve, reject){
    // The entire AJAX operation inside here
  });
}

Your xhr handler only checks for state 4. State 4 only indicates the operation succeeded but does not indicate that you encountered HTTP errors like 400 or 500. Additionally, if the xhr did fail, there's no way of knowing from the Promise since you did not do a reject. Check for HTTP success and failure, do a resolve and reject respectively.

if(ajax.readyState === 4 && ajax.status === 200){
  resolve(ajax.responseText);
} else {
  reject(ajax.responseText);
}


Getting the unique factors of a number recursively I have written the below code to get the unique factors. Please offer suggestions for better results.

#Python program to print out all ways to multiply smaller integers
#that equal the original number, without repeating sets of factors

def print_factors_list(dividend, factorstring, predivisor):
"""This function takes a number and prints the factors"""

   divisor = dividend - 1
   #Looping to get the factors
   for i in range(divisor, 1, -1 ):

      if dividend % i != 0:
        continue

      if i > predivisor:
        continue
      #Get the quotient for the multipying number
      quotient = dividend / i

      if quotient <= i:
         if quotient <= predivisor:
            print factorstring + str(i) + "*" + str(quotient)
      print_factors_list(quotient, str(factorstring) + str(i) + "*", i)

#Check if the number is greater than 0
def print_factors(x):
# Check if the number is greater than 0
    if (x < 0):
        print "Enter a positive integer"
    #Check if the number is 0
    elif (x == 0):
        print "Enter a positive integer"
    #Go to the function print_factors_list for further evaluation
    else:
    #Print the default factors(the number itself and 1)
       print str(x) + "*" + str(1)
       print_factors_list(x, "", x )

#Take input from the user
num = int(input("Enter a number: "))
#Call the function with the number given by the user
print_factors(num)

An alternative (and probably more efficient) approach would be to derive all prime factors (hint: you could use divmod) and then create each of the possible factorings from that (hint: you could use itertools combinations).

Some notes:

If  you extract prime factors from smallest to largest, the remainder will always either be prime or be a composite with a smallest prime factor greater than or equal to the last one found.
One can also use the fact that all of the prime numbers except 2 are odd numbers to step through quickly.
The largest possible prime factor of any integer \$n\$ may be discovered by examining no more than \$\sqrt{n}\$ potential factors.

You might want to peek at this question for a Python-based prime factoring program.

Getting all factors from prime factorization

Starting with an example 60, we can easily determine that its prime factorization p = [2, 2, 3, 5].  Clearly, this is one possible factorization, which can easily be output in Python 3 as

print(*p, sep=' * ')

If you're using 2.6 or 2.7, you can still use this neat syntax, but you'll have to do this instead

from __future__ import print_function
print(*p, sep=' * ')

This prints:

2 * 2 * 3 * 5

So now all you need to do is create all possible unique lists of factors.  So for each two-factor combination, you could use this:

for i in itertools.permutations(p,len(p)):
    print(*[i[0],reduce(lambda x,y:x*y,i[1:])],sep=' * ')

That creates a list of all such two-factor combinations with exactly one prime factor, but they are not unique.  Because there is a repeated factor of 2 in the list, the product "2 * 30" is printed twice.  One way to handle this is to store each factor as it's used and suppress printing of non-unique factors.

Similarly, all non-unique 3-factor combinations with one prime factor:

for i in itertools.permutations(p,len(p)):
    print(*[i[0],i[1],reduce(lambda x,y:x*y,i[2:])],sep=' * ')

Or alternatively:

for i in itertools.permutations(p,len(p)):
    print(*(list(i[:2])+[reduce(lambda x,y:x*y,i[2:])]),sep=' * ')

This should give you enough of a clue as to how you might generate all possible factorizations.  A recursive approach is probably even better here.

Also, as an incidental note, factorings with 1 are typically omitted as trivial.


Eliminate for loops in numpy implementation I have the following dataset in numpy

indices | real data (X)    |targets (y)
        |                  |
0   0   | 43.25 665.32 ... |2.4      } 1st block
0   0   | 11.234           |-4.5     }
0   1     ...               ...      } 2nd block
0   1                                }
0   2                                } 3rd block
0   2                                }
1   0                                } 4th block
1   0                                }
1   0                                }
1   1                       ...
1   1
1   2
1   2
2   0
2   0
2   1
2   1
2   1
...

Theses are my variables

idx1 = data[:,0]
idx2 = data[:,1]
X = data[:,2:-1]
y = data[:,-1]

I also have a variable W which is a 3D array.

What I need to do in the code is loop through all the blocks in the dataset and return a scalar number for each block after some computation, then sum up all the scalars, and store it in a variable called cost. Problem is that the looping implementation is very slow, so I'm trying to do it vectorized if possible. This is my current code. Is it possible to do this without for loops in numpy?

IDX1 = 0
IDX2 = 1

# get unique indices
idx1s = np.arange(len(np.unique(data[:,IDX1])))
idx2s = np.arange(len(np.unique(data[:,IDX2])))

# initialize global sum variable to 0
cost = 0
for i1 in idx1s:
    for i2 in idx2:

        # for each block in the dataset
        mask = np.nonzero((data[:,IDX1] == i1) & (data[:,IDX2] == i2))

        # get variables for that block
        curr_X = X[mask,:]
        curr_y = y[mask]
        curr_W = W[:,i2,i1]

        # calculate a scalar
        pred = np.dot(curr_X,curr_W)
        sigm = 1.0 / (1.0 + np.exp(-pred))
        loss = np.sum((sigm- (0.5)) * curr_y)

        # add result to global cost
        cost += loss

Here is some sample data

data = np.array([[0,0,5,5,7],
                 [0,0,5,5,7],
                 [0,1,5,5,7],
                 [0,1,5,5,7],
                 [1,0,5,5,7],
                 [1,1,5,5,7]])
W = np.zeros((2,2,2))
idx1 = data[:,0]
idx2 = data[:,1]
X = data[:,2:-1]
y = data[:,-1]

That W was tricky... Actually, your blocks are pretty irrelevant, apart from getting the right slice of W to do the np.dot with the corresponding X, so I went the easy route of creating an aligned_W array as follows:

aligned_W = W[:, idx2, idx1]

This is an array of shape (2, rows) where rows is the number of rows of your data set. You can now proceed to do your whole calculation without any for loops as:

from numpy.core.umath_tests import inner1d
pred = inner1d(X, aligned_W.T)
sigm = 1.0 / (1.0 + np.exp(-pred))
loss = (sigm - 0.5) * curr_y
cost = np.sum(loss)


Dijkstra's algorithm using priority queue running slower than without PQ I need to implement dijkstra's algorithm and I've done so using this Wikipedia page. I've done it both with priority queue and without.

Both versions work 100% correct, however I need the faster one (priority queue one). The problem is that it isn't faster. My tests show "faster" one runs in more than double the time that "slower" one does, it shouldn't do that. What am I doing wrong?

//more primitive version that in fact runs faster (it shouldn't)

//source is starting node, adj adjacency list, Road represents one edge
private static int dijkstra (int source, ArrayList<Road>[] adj) {
    HashSet<Integer> vertices = new HashSet<>();

    int[] dist = new int[adj.length];
    int[] prev = new int[adj.length];

    for (int i = 0; i < adj.length; i++) {
        dist[i] = Integer.MAX_VALUE;
        prev[i] = Integer.MAX_VALUE;
        vertices.add(i);
    }

    dist[source] = 0;

    while (!vertices.isEmpty()) {
        int currentPathLen = Integer.MAX_VALUE, current = -1;
        for (int v: vertices) {
            if (dist[v] < currentPathLen) {
                current = v;
                currentPathLen = dist[current];
            }
        }

        vertices.remove(current);

        for (Road v: adj[current]) {
            int alt = dist[current] + v.distance;

            if (alt < dist[v.end]) {
                dist[v.end] = alt;
                prev[v.end] = current;
            }
        }
    }
}

//this one should run faster

//same variables as primitive version, except for Node,
//which is made only for PQ prioritizing
private static int improvedDijkstra(int source, ArrayList<Road>[] adj) {
    PriorityQueue<Node> vertices = new PriorityQueue<>();

    int[] dist = new int[adj.length];
    int[] prev = new int[adj.length];

    dist[source] = 0;

    for (int i = 0; i < adj.length; i++) {
        if (i != source) {
            dist[i] = Integer.MAX_VALUE;
            prev[i] = Integer.MAX_VALUE;
        }

        vertices.add(new Node(i, dist[i]));
    }

    while (!vertices.isEmpty()) { //O(n)
        Node u = vertices.poll(); //this should have O(logn)

        for(Road v: adj[u.value]) {  // I suspect the problem is this loop, but it is same
                                     // as in Wikipedia page

            int alt = dist[u.value] + v.distance;

            if (alt < dist[v.end]) {
                dist[v.end] = alt;
                prev[v.end] = alt;

                vertices.add(new Node(v.end, alt)); //this should have O(logn)
            }
        }
    }

I think the problem is that you have more than one instance of a node in the priority queue at a given time.

In the relax operation:

 if (alt < dist[v.end]) {
     dist[v.end] = alt;
     prev[v.end] = alt;

     vertices.add(new Node(v.end, alt)); //this should have O(logn)
 }

You should remove the vertex before adding it or implement a indexed priority queue with a decreased key operation like this one. I'm not sure if java remove operation is \$O(ln(n))\$ thought. Like this:

if (alt < dist[v.end]) {
     dist[v.end] = alt;
     prev[v.end] = alt;
     Node newNode = new Node(v.end, alt);
     vertices.remove(newNode);
     vertices.add(newNode);
 }

Note: You need to override equals for the node.

With the indexed priority queue you would create and add the new node if the queue doesn't contains it, or do decreased key otherwise.

If you do it like this you will lazily add each vertex to the queue so you don't need to add them all at the beginning here:

   dist[source] = 0;
   for (int i = 0; i < adj.length; i++) {
        if (i != source) {
            dist[i] = Integer.MAX_VALUE;
            prev[i] = Integer.MAX_VALUE;
        }

        vertices.add(new Node(i, dist[i])); // <--here
    }

Also you can remove the inner if in the loop like this:

   for (int i = 0; i < adj.length; i++) {
       dist[i] = Integer.MAX_VALUE;
       prev[i] = Integer.MAX_VALUE;
    }
    dist[source] = 0;
    vertices.add(new Node(source, 0));


Read framed nested sensor data from memory stream I'm implementing a TCP client to read data from a sensor. The sensor sends messages to connected clients framed with a start sequence, messages, checksum and end sum bytes. Because the sensor is installed at a remote location, I'm currently reading previously captured network data from a pcap dump into a MemoryStream. The TCP data is simulated from a capture file using bittwist (not real-time client-server communication).

Please advise on how to improve the method below that reads TCP messages including framing bytes, through a MemoryStream since I can't use NetworkStream? Is my checksum calculation correct? The sensor documentation indicates that to calculate it you must include all payload data except starting and ending sequence.

I have two while loops, one to first check start sequence (frame start), once done.

The second loop checks for the checksum value, end sequence (frame end) and if not, treats this as a message payload and reads from the stream until a valid end sequence (frame end) is found. Each (n) payload starts with a 3 bytes header (2 bytes identifier + 1 byte length of the succeeding data) and then the actual data.

private static void Decode(MemoryStream memoryStream)
    {
        var frameStartSequenceBuffer = new byte[] { 0xCA, 0xCB, 0xCC, 0xCD };
        var frameChecksumBuffer = new byte[1];
        var frameEndSequenceBuffer = new byte[] { 0xEA, 0xEB, 0xEC, 0xED };
        var startSequenceBuffer = new byte[4];
        var endSequenceBuffer = new byte[4];
        var messages = new List<byte[]>();
        var read = 0;

        while (read < startSequenceBuffer.Length)
        {
            read += memoryStream.Read(startSequenceBuffer, 0, startSequenceBuffer.Length);

            if (Helpers.ByteArrayCompare(startSequenceBuffer, frameStartSequenceBuffer))
                break;
        }

        while (memoryStream.Capacity > read)
        {
            read += memoryStream.Read(frameChecksumBuffer, 0, frameChecksumBuffer.Length);
            read += memoryStream.Read(endSequenceBuffer, 0, endSequenceBuffer.Length);

            if (Helpers.ByteArrayCompare(endSequenceBuffer, frameEndSequenceBuffer))
                break;

            var readPayload = frameChecksumBuffer.Concat(endSequenceBuffer).ToArray();
            var messageBuffer = new byte[3 + endSequenceBuffer[1]];

            readPayload.CopyTo(messageBuffer, 0);
            read += memoryStream.Read(messageBuffer, readPayload.Length, messageBuffer.Length - readPayload.Length);

            messages.Add(messageBuffer);
        }

        var calculatedChecksum = Helpers.CalculateChecksumBitwise(messages.SelectMany(o => o).Concat(frameChecksumBuffer).ToArray());

        if (calculatedChecksum != frameChecksumBuffer[0])
        {
            Console.WriteLine("Messages: {0} - Corrupt", messages.Count);
            return;
        }

        Console.WriteLine("Messages: {0} - Valid", messages.Count);
    }

Each TCP packet from the sensor encapsulates an array of (n) messages delimited with a 3 bytes headers.

The checksum is calculated from all data without the start sequence and the end sequence. The Checksum is a simple XOR Assignment of all n data bytes. Below is the checksum calculation method that I've implemented.

public static byte CalculateChecksumBitwise(IList<byte> byteToCalculate)
    {
        var checksum = 0;

        for (var i = 0; i < byteToCalculate.Count - 1; i++)
        {
            checksum = checksum ^ byteToCalculate[i];
        }

        return (byte)checksum;
    }

I think the first loop is somewhat redundant, because if read < startSequenceBuffer.Length after the first pass, the input memoryStream is shorter than the startSequenceBuffer and then there are no messages to follow anyway. Instead of the first loop you can do:

       int read = memoryStream.Read(startSequenceBuffer, 0, startSequenceBuffer.Length);

        if (!Helpers.ByteArrayCompare(startSequenceBuffer, frameStartSequenceBuffer))
            throw new InvalidDataException();

If the first loop is meant as a search for the start sequence, you'll have to find another stop condition, because it will stop after the first pass, unless the stream is shorter then startSequenceBuffer.

  while (memoryStream.Capacity > read) {...}

Be aware that MemoryStream.Capacity >= MemoryStream.Length, so the second loop can unintentionally try to read beyond memoryStream.Length which will cause an infinite loop because trying to read from a position greater than Length will add nothing to read. Use memoryStream.Length instead.

The meat of the second loop is a little confusing without any data to test on. At first sight it seems to work though. It seems odd that frameChecksumBuffer is written to more than once, because you only use it in one test at the end of the method, but maybe I just misunderstand the name or the format of the stream?


Groovy optimize method to convert JSON to map I have this method which creates a map from a JSON request parameter. Right now I've hard-coded JSON keys to convert it to map. Can it be converted to a generic method which converts a request's JSON object to a map?

Request:

Content-Disposition: form-data; name="invoice_903"
{"id":"903","parcelId":18,"referenceId":"T44428"}`

Groovy method:

def getRequestParameterMap(def request) {
        def params = request.getParameterNames()
        def requestParamMap = [:]
        while (params.hasMoreElements()) {
            String fieldName = (String) params.nextElement();
            if (fieldName.startsWith("invoice_")) {
                JSONObject obj = JSONUtil.parseJSONObject(request.getParameter(fieldName));
                def id = obj.get("id").toString()
                def parcelId = obj.get("parcelId").toString()
                def referenceId = obj.get("referenceId").toString()
                requestParamMap["id"] = id
                requestParamMap["parcelId"] = parcelId
                requestParamMap["referenceId"] = referenceId
            }
        }
        requestParamMap
    }

In order to convert String to Map, use JsonSlurper:

new JsonSlurper().parseText(requestString)