VSTO Write Dictionary To Range Extension Method I was hoping to receive some criticism on this block of code. I wrote it to write the contents of a dictionary to excel range in VSTO. It works well. I am interested to see how people could improve it.  What would you do differently to make it more expressive, efficient?

public static void WriteToRange(this Dictionary<string, object[]> DicToRange, excel.Range toRange)
    {
       object[,] storeDictionaryData = new object[DicToRange.Max(maxValues=> maxValues.Value.Length),DicToRange.Count];

       int keyCounter=0;
       int valueCounter =0;

       foreach (var key in DicToRange.Keys)
       {
         if (keyCounter < storeDictionaryData.GetLength(1))
           {
               foreach (var value in DicToRange[key])
               {
                   if (valueCounter < storeDictionaryData.GetLength(0))
                   {
                       storeDictionaryData[valueCounter, keyCounter] = value;
                       valueCounter++;
                   }
               }

               keyCounter++;
               valueCounter = 0;
           }
       }

       dynamic rangeToFill=toRange.get_Resize(storeDictionaryData.GetLength(0), storeDictionaryData.GetLength(1));
       rangeToFill.Value = storeDictionaryData;
    }

You have a lot of unnecessary vertical spacing which should be removed to improve readability.

Based on the naming guidelines input parameter should be named using camelCase casing.

Because WriteToRange() can be called also as a normal static method you need to check both input parameters for null.

The storeDictionaryData is basically a object[columns, rows] this should be reflected by the counter variables. You can just rename them valueCounter -> column, keyCounter -> row.

You don't need the checks keyCounter < storeDictionaryData.GetLength(1) and valueCounter < storeDictionaryData.GetLength(0) because at the point where you create the array you have taken care of the upper bounds of both dimensions. Both conditions won't evaluate to false.

toRange is poorly named. Consider to name it destinationRange or simply destination. Then a good pattern would be also to rename DictToRange to source.

You should allow your variables to breathe by adding spaces between variables and operators.

Taking the above points into account we can refactor the method to

public static void WriteToRange(this Dictionary<string, object[]> source, excel.Range destination)
{
   if (source == null) { throw new ArgumentNullException("source"); }
   if (destination == null) { throw new ArgumentNullException("destination"); }
   if (source.Count == 0) { throw new ArgumentOutOfRangeException("source"); }

   object[,] storeDictionaryData = new object[source.Max(maxValues => maxValues.Value.Length), source.Count];

   int column = 0;
   int row = 0;

   foreach (var key in source.Keys)
   {
       foreach (var value in source[key])
       {
           storeDictionaryData[column, row] = value;
           column++;
       }

       row++;
       column = 0;
   }

   dynamic rangeToFill = destination.get_Resize(storeDictionaryData.GetLength(0), storeDictionaryData.GetLength(1));
   rangeToFill.Value = storeDictionaryData;
}


Casting const pointer to non-const pointer when using struct iovec struct iovec is defined in <uio.h> by the following way:

/* Structure for scatter/gather I/O.  */
struct iovec
  {
    void *iov_base; /* Pointer to data.  */
    size_t iov_len; /* Length of data.  */
  };

Now I do ugly casting from const void* to void*:

int send_frame_once( int fd, const struct iovec* iov, size_t iovlen );

int send_frame( int fd, const void* buffer, size_t len )
{
    struct iovec iov;
    iov.iov_base = (void*)buffer;       /* const iov cast */
    iov.iov_len  = len;
    return send_frame_once( fd, &iov, 1 );
}

Is that necessary kludge?
Or should I remove const keyword from declaration of send_frame function. Like this:

int send_frame( int fd, void* buffer, size_t len )

I think you should keep it. const is not only meant for the compiler, but also for anybody reading your code. Even with the void* cast, you're still saying to readers "Trust me, I'm not touching your buffer!" and this is a valuable information that should not be discarded.


Adding push to array inside the class that is being pushed I used to mostly do this:

class Person
  constructor : (@parent, @data)->
    @index = @parent.length

class People
  constructor : ->
    people = []

    for data in database.people
      people.push new Person(people, data)

Lately I have been trying the following:

class Person
  constructor : (@data)->

  pushTo : (list)->
    this.index = list.length
    list.push this

class People
  constructor : ->
    people = []

    for data in database.people
      person = new Person(data)
      person.pushTo people

So was wondering if there is any cons about doing it the second way. The reason I am asking is that it might not be the responsibility of Person to add himself to the list.

The reason I am asking is that it might not be the responsibility of Person to add himself to the list.

I think you should ask youself a question "Why am I doing this?" Why do you need to keep the @index inside a Person?

Is it just an id? If so why don't you use _.uniqueId('person') from Underscore JS? That would be much cleaner.
Do you use it for ordering so you could for example sort an array of people by it? If so uniqueId would still suit your needs well.
Do you use it to search for person by it's index in the list later on? Basically I can envision two kinds of data processing.

You iterate over a list of people. In this case you'll be fine with Underscore's each method:

_.each people, (person, index) ->
    // your code here     ^----- see that? ;)

You got a person data from some external source and you want to reuse the associated Person object. In this case you should still use an ID and search for your object with find or select methods.

Some notes:

Both your solutions are totally fine if you certain that they don't complicate the code of your application. They also apply in cases when your collection is very large and find takes too much time. BUT you can speed up selection on later invocations:

indexFor = (targetPerson) ->
  unless person.index // we search for index once and then store it for later use
    _.find people, (person, index) ->
      person.index = index if found = (person.id == targetPerson.id)
      found
  targetPerson.index

uniqueId uses internal counter so the ID is always increasing. It cannot be reset which may not be always convenient. For example if you need to save your collection and reload it later you won't be able to tell the generator to increase the counter accordingly. In that case I would still go with uniqueId for presentation code but use some other ID generation scheme for persistence. I would go with UUIDs:  but you can also try timestamps though I wouldn't recommend it since generally time is way too difficult to get right.
You should definitely check out other methods in Underscore library. Whenever you encounter a kind of low-level task you should ask if Underscore does it for you already. If it doesn't check jQuery or whatever library you use for UI. If you're still out of luck ask Google or StackOverflow. I'm sure you're great coder and you can solve many of those tasks yourself but why should you spend your time and effort reinventing stuff instead of creating something new? Know your libraries and use them. It's like a new language or a new text editor - at first it's painful but it pays off later on.


Refactor jaccard similarity the "Scala way" I'm trying to pick Scala up. This is a simple heuristic that checks a similarity value between two sets. I've done this a million times in Java or Python. The function works, but I'm certain I am not doing this "the Scala way".

val a : List[Int] = List(1,1,2,2,3)
val b : List[Int] = List(1,2,3,4,5)

def calculateBags(a: List[Int], b: List[Int]): Double = {
    val a_counts = a.groupBy(x=>x).map(y=> y._1->y._2.size)
    val b_counts = b.groupBy(x=>x).map(y=> y._1->y._2.size)
    var count = 0
    a_counts.foreach{ x=>
        var k = x._1
        var t = x._2
        if (b_counts.contains(k)) {
            count+= Math.min(t,b_counts(k))
        }
    }
    (count.toDouble)/(a.size+b.size)
}

Idiomatic Scala Style

When defining a val, var, or a function use camelCase.

    // :)
    val aCounts = ...

    // :(
    val a_counts = ...

When using an infix operator add a space on both sides of it.

    // :)
    x => x
    i -> j
    foo * boo

    // :(
    x=>x
    i->j
    foo*boo

Code

Here is your code with all of the changes I would make included (explanation follows):

    import Math.min

    def calculateBags(a: List[Int], b: List[Int]): Double = {
        val countsByElem =
            (xs: List[Int]) =>
                xs.groupBy(elem => elem).map { case (e, xss) => e -> xss.length }

        val aCounts = countsByElem(a)
        val bCounts = countsByElem(b)

        def addMin(count: Int, x: (Int, Int)): Int = {
            val (k, t) = x
            val v = bCounts getOrElse(k, 0)
            count + min(v, t)
        }

        val len = (a.length + b.length).toDouble

        (0 /: aCounts)(addMin) / len
    }

The first thing I did was define a function literal, countsByElem. I mostly did this to show you that such expressions are possible and sometimes very useful. The advantage in this case is that instead of duplicating the same logic in multiple places, we just define a function and call it as necessary. If we then need to make a change to the logic we can do so in one place. Another advantage is that a future reader of your code can instantly tell aCounts and bCounts are calculated with the same set of operations (though with different lists). A disadvantage is that defining this function literal adds more lines of code to your function overall. But we will make up for that later :)
Next, I've defined a function which for lack of a better name I've called addMin. The parameters have the same names as you used in your original code: count is the accumulator and x is the ith  element of aCounts. The notable features of this function are:

In the first line we unpack the tuple value x with some syntactic sugar
The next line (starting val v = ...) attempts to retrieve a value from the bCounts map based on the key k. If there is no value associated with k in bCounts the integer value 0 is returned instead.

Lastly we utilize a foldLeft which in the code is designated by the /:. It is a bit hard to concisely explain the set of fold methods that Scala offers. But a nice tutorial on them can be found here. Brief explanation: we pass each element in aCounts along with an accumulator to addMin. If we were to 'unroll' some arbitrary fold-left it might look something like this:

// Scala-esque psuedocode to explain a fold ...
val foldResult = {
    val res0 = func(0,    xs(0))
    val res1 = func(res0, xs(1))
    val res2 = func(res1, xs(2))
    // ...
    func(penultimateResult, xs.last)
}

Anyway, hope this helps!


Binary Search Tree implementation with unique pointers I have implemented a binary search tree using templates and unique_ptr in C++ 11. At present, only insertion and deletion are implemented. Please provide your feedback for improvements.

#include <iostream>
#include <memory>

template<typename T>
class Btree {
public:
    void insert(T data)
    {
        _insert(root, data);
    }
    void traverse(void (*func)(T data))
    {
        _traverse(root, func);
    }
    void del(T data)
    {
        _del(root, data);
    }
private:
    struct node {
        T data;
        std::unique_ptr<node> left, right;
        node(T data): data(data), left(nullptr), right(nullptr) {}
    };

    std::unique_ptr<node> root;
    void _insert(std::unique_ptr<node>& curr, T data);
    void _del(std::unique_ptr<node>& curr, T data);
    void _traverse(std::unique_ptr<node>& curr, void (*func)(T data));
    T _findmin(std::unique_ptr<node> &curr);
};

template<typename T>
void Btree<T>::_insert(std::unique_ptr<node>& curr, T data)
{
    if (curr == nullptr) {
        curr.reset(new node(data));
        return;
    }

    if (data < curr->data)
        _insert(curr->left, data);
    else
        _insert(curr->right, data);
}
template<typename T>
T Btree<T>::_findmin(std::unique_ptr<node>& curr)
{
    if (curr && curr->left == nullptr)
        return curr->data;
    return _findmin(curr->left);
}
template<typename T>
void Btree<T>::_del(std::unique_ptr<node>& curr, T data)
{
    if (curr == nullptr)
        return;
    if (data < curr->data)
        _del(curr->left, data);
    else if (data > curr->data)
        _del(curr->right, data);
    else {
        // if one child is nullptr or both child are nullptr
        if (curr->left == nullptr) {
            auto &p = curr->right;
            curr.reset(p.release());
        }
        else if (curr->right == nullptr) {
            auto &p = curr->left;
            curr.reset(p.release());
        }
        //if child is non leaf node
        else {
            T temp = _findmin(curr->right);
            curr->data = temp;
            _del(curr->right, temp);
        }
    }
}
template<typename T>
void Btree<T>::_traverse(std::unique_ptr<node>& curr, void (*func)(T data))
{
    if (curr == nullptr)
        return;
    _traverse(curr->left, func);
    func(curr->data);
    _traverse(curr->right, func);
}

Observations
You pass data by value. This is fine for small data types like int. But imagine that T is some huge object that is very expensive to copy. You should think of passing the data by reference to avoid any intermediate copies. When you get a bit more advanced think about passing by r-value reference to allow you to move the objects.
For your traversal you pass a function pointer. This is a bit limiting. Normally you would templateze the function to allow you to pass any function like object (function pointer/functor/lambda/std::function) etc.
Yes you need recursion when traversing trees. But you don't need it everywhere. There are a couple of places where a simple loop would be sufficient.
I can see why you use std::unique_ptr. But in my view a tree is a container and should manage its own memory. So I would have simply used Node* inside data. The code is not that much harder to write in this context. BUT I don't have any real issue with std::unique_ptr.
Code
Insert by const reference
void insert(T const& data)
{
    _insert(root, data);
}
_insert(std::unique_ptr<node>& curr, T const& data)
{
    if (curr.get() == nullptr) {
        curr.reset(new node(data));
        return;
    }

    auto& next = (data < curr->data) ? curr->left : curr->right;
    _insert(next, data);
}

Insert by r-Value reference
void insert(T&& data)
{
    _insert(root, std::forward<T>(data));
}
_insert(std::unique_ptr<node>& curr, T&& data)
{
    if (curr.get() == nullptr) {
        curr.reset(new node(data));
        return;
    }

    auto& next = (data < curr->data) ? curr->left : curr->right;
    _insert(next, std::forward<T>(data));
}

Emplace into node.
// This calls T constructor only when you construct the node itself.
template<typename... Args>
void emplace(Args const&... args)
{
    _emplace(root, args...));
}
template<typename... Args>
_emplace(std::unique_ptr<node>& curr, Args const&... args)
{
    if (curr.get() == nullptr) {
        curr.reset(new node(args...));
        return;
    }

    auto& next = (data < curr->data) ? curr->left : curr->right;
    _insert(next, args...);
}
template<typename... Args>
node::node(Args const&... args)
    : data(args...)  // Data of type T constructed in place
    , left(nullptr)
    , right(nullptr)
{}

Using a function like object to traverse the tree:
template<typename F>
void traverse(const & action)
{
    _traverse(root, action);
}
template<typename F>
void _traverse(std::unique_ptr<node>& curr, F const& action);

// outside class

template<typename T>
template<typename F>
void Btree<T>::_traverse(std::unique_ptr<node>& curr, F const& action)
{
    if (curr == nullptr)
        return;
    _traverse(curr->left, action);
    action(curr->data);
    _traverse(curr->right, action);
}

int main()
{
     Btree<int>  tree;
     tree.traverse([](int x){std::cout << x << " ";});
}


Is this predicate valid to delete single shared_ptr's? I wrote a predicate used in a remove_if call that deletes shared_ptr's of type StemmedSentence from an vector of sentences.

The predicate:

class EraseSentenceIf {
    ArrayStemmedSnippet * m_ass;

public:
    EraseSentenceIf(ArrayStemmedSnippet *ass)
    : m_ass(ass) {
    }

    bool operator()(const std::shared_ptr<
        ArrayStemmedSnippet::StemmedSentence>& s) {
        std::shared_ptr<ArrayStemmedSnippet::StemmedSentence> tmp = s;

        // --- set StemmedSentnce object in ArrayStemmedSnippet class
        s->setParent(m_ass);

        // --- if true delete this sentence)
        if (s->trimStopWords()) {
            tmp.reset();

            return true;
        }

        return false;
    }
};

The remove_if call:

EraseSentenceIf esi(this);
sentences.erase(
    std::remove_if(
        sentences.begin(), sentences.end(), esi),
    sentences.end()
);

Declaration:

std::vector<shared_ptr<StemmedSentence> > sentences;

The construction of the sentences objects looks like this:

sentences.push_back(shared_ptr<StemmedSentence>(
    new StemmedSentence(index, i - 1 )));

The code seems to run fine, valgrind / gdb does not moan. I just want to get sure that I handle  the deletion (or release) of the shared_ptr in a correct way. Can somebody please confirm this? Maybe I can improve something or I overlooked an important point. Thanks for your comments!

The copy of the pointer is completely unnecessary: as long as the code is not sharing shared_ptrs among threads (which is really, really silly), taking a shared_ptr argument by reference ensures it lives at least until the function finishes executing.
Even if the copy was necessary, passing by reference to immediately make a copy is a pessimisation as it forbids moves. Don't pass shared_ptr by reference if you're going to copy it.
The pointer in the vector will be destroyed by erase, there's no need to manually reset anything.
A predicate that mutates its argument is bound to raise eyebrows. Depending on the semantics of setParent it may or may not be problematic, but it is something I'd avoid if I could.


Parser to extract calendar data I'm wondering how I can improve this class, for instance:

Should local variables such as allInput that are passed in to more than one function be instance variables?
Should functions like determineMeetingFormat and parseParticipants be called directly into the Meeting constructor or be assigned to local constants that clearly defines what's being returned?
Would it be better to make location and description optional Strings rather than empty strings?
Is making all my functions and instance variables static the best way to make this behave like a static class?

/**

/**
     Parses calendar entry to create a Meeting object.

 - parameter calendarEvent: The calendar entry we want to parse
 - returns: A Meeting Object with the relevant information extracted from the calendar entry.
 */
static func parse(calendarEvent: EKEvent) -> Meeting {
    let location = calendarEvent.location ?? ""
    let description = calendarEvent.notes ?? ""

    let allInput = "\(calendarEvent.title)\n\(location)\n\(description)"
    let parsedHostCodes = parseHostCode(from: allInput)
    let parsedPasscodes = parseParticipantCode(from: allInput, hostCodes: parsedHostCodes)
    let parsedPhoneNumbers = parsePhoneNumber(from: allInput, codes: parsedPasscodes + parsedHostCodes)
    let meetingFormat = determineMeetingFormat(input: allInput, parsedPhoneNumbers: parsedPhoneNumbers)

    return Meeting(
        UUID: calendarEvent.eventIdentifier,
        title: calendarEvent.title,
        description: description,
        location: location,
        host: retrieveHost(from: calendarEvent.organizer),
        startTime: calendarEvent.startDate,
        endTime: calendarEvent.endDate,
        allday: calendarEvent.isAllDay,
        passcodes: parsedPasscodes,
        hostcodes: parsedHostCodes,
        phoneNumbers: parsedPhoneNumbers,
        attendees: parseParticipants(from: calendarEvent.attendees),
        isAtt: allInput.contains(pattern: attRegex),
        meetingFormat: meetingFormat)
}

Static constructors generally do lead to the problems you have encountered, where the implementation tends to be procedural, and complexity increases as the constructed classes become more intricate.

This can be made more manageable by using a builder pattern, which is implemented as a class that exists for the purpose of constructing the final class. This allows you to maintain an object-oriented approach.

The builder class can be used in the static parse function, so the interface can stay the same if needed.

Ideally, the parse function should be implemented as a convenience initialiser since it is constructing an object instance.

To answer your questions:

  Should local variables such as allInput that are passed in to more than one function be instance variables?

Yes. These should be instance variables of the builder. See the example below.

  Should functions like 'determineMeetingFormat' and 'parseParticipants' be called directly into the Meeting constructor or be assigned to local constants that clearly defines what's being returned?

Yes. Local constants/variables are self documenting to some degree and hence improve readability. They also give you an easy reference to trace during debugging if needed.

  Would it be better to make location and description optional Strings rather than empty strings?

Yes. By making these optionals the compiler can help you in those places where you might need to check if the string exists. The alternative is to use isEmpty which you can only see at runtime / unit tests. Compile time checking gives more immediate results.

  Is making all my functions and instance variables static the best way to make this behave like a static class?

Not always. Static methods behave like singletons in many ways, which has many disadvantages, such as being hard to test and debug. They are often provided as conveniences which delegate to an equivalent instance method. You are essentially creating a constructor, which would be better implemented as a convenience initializer (i.e. init).

Example 1:

The example below shows one way that the builder pattern could be implemented:

Builder class

class MeetingParser {
    let input: String

    // Instance variables for the parsed values.
    // These can be optionals for more safety.
    var parsedHostCodes: [HostCode]!
    var parsedPassCodes: [PassCode]!
    var parsedPhoneNumbers: [PhoneNumber]!
    var meetingFormat: MeetingFormat!
    var participants: [Participant]!
    var isAtt: Bool
    var host: String!

    init(input: String) {
        self.input = input
    }

    func parse() {
        // The functions below would access the instance variables.
        parsedHostCodes = parseHostCode()
        parsedPasscodes = parseParticipantCode()
        parsedPhoneNumbers = parsePhoneNumber()
        meetingFormat = determineMeetingFormat()
        participants = parseParticipants()
        isAtt = input.contains(pattern: attRegex)
        host = retrieveHost()
    }
}

Meeting convenience initialiser

extension Meeting {
    init(event: EKEvent) {
        let allInput = "\(event.title)\n\(event.location)\n\(event.description)"
        let parse = MeetingParse(input: allInput)
        parser.parse()
        self.UUID = event.eventIdentifier
        self.title = event.title
        self.description = event.description
        self.location = event.location
        self.host = parser.host
        self.startTime = event.startDate
        self.endTime = event.endDate
        self.allday = event.isAllDay
        self.passcodes = parser.parsedPasscodes
        self.hostcodes = parser.parsedHostCodes
        self.phoneNumbers = parser.parsedPhoneNumbers
        self.attendees = parser.participants
        self.isAtt = parser.isAtt
        self.meetingFormat = parser.meetingFormat
    }
}

Example 2

Below is a second example, also using a builder class, although this one assumes the properties of the Meeting class are implicit optionals, which simplifies the construction to some extent. This comes at the cost of reducing safety as it potentially allows nils to be unwrapped which would result in a runtime crash.

class MeetingParser {
    let event: EKEvent
    let meeting: Meeting

    init(event: EKEvent, meeting: Meeting) {
        self.event = event
        self.meeting = meeting
    }

    func build() {
        meeting.UUID = event.eventIdentifier
        meeting.title = event.title
        meeting.description = event.description
        meeting.location = event.location
        meeting.host = retrieveHost()
        meeting.startTime = event.startDate
        meeting.endTime = event.endDate
        meeting.allday = event.isAllDay

        // Class methods re-use the values in meeting.
        // Could instead use instance variables to hold intermediate values.
        meeting.parsedHostCodes = parseHostCode()
        meeting.parsedPasscodes = parseParticipantCode()
        meeting.parsedPhoneNumbers = parsePhoneNumber()
        meeting.meetingFormat = determineMeetingFormat()
        meeting.participants = parseParticipants()
        meeting.isAtt = input.contains(pattern: attRegex)
    }
}

extension Meeting {
    // Meeting properties are defined as simplicity unwrapped optionals:
    // var title: String! = nil
    init(event: EKEvent) {
        let builder = MeetingBuilder(event: event, meeting: self)
        builder.build()
    }
}

Example usage:

let meeting = Meeting(event: event)

Other advice:

Try to reduce the overall size of the Meeting class to simplify the parser. Try to encapsulate related data into smaller classes. As an example, group the time and date information into an object to the properties for startTime, endTime and isAllDay into a single property:

enum MeetingTime {
    case allDay
    case range(start: Date, end: Date)
}

class Meeting {
    // ... other properties
    var time: MeetingTime?
    // ... other properties
}

let meeting: Meeting
meeting.time = .allDay // All day meeting.
meeting.time = .range(start: .now, end: .now) // Meeting between 2 times.


Evaluating formulas greater than 255 characters Recently I was receiving Error 2015 when trying to use Application.Evaluate, then I realized it was because my formula was greater than 255 characters. As a workaround, I came up with this UDF:

Function AdvancedEvaluate(eformula As String)

    ThisWorkbook.Worksheets("Scripting Worksheet").Range("A1").Formula = eformula
    AdvancedEvaluate = ThisWorkbook.Worksheets("Scripting Worksheet").Range("A1").Value
    ThisWorkbook.Worksheets("Scripting Worksheet").Range("A1").ClearContents

End Function

Which I use in place of all of my Application.Evaluate. However, I'm not satisfied with this solution... is my solution sufficient, or is there a better way to evaluate a formula of greater than 255 characters?

The function is implicitly public, takes an implicitly ByRef parameter that has no reason to not be passed ByVal, and returns an implicit Variant that should be explicit.

It's also side-effecting, which makes it unusable as an actual UDF.

ThisWorkbook.Worksheets("Scripting Worksheet") suggests the procedure is using a purposely-made bogus (hidden?) sheet just for that. So why does that sheet need to be dereferenced 3 times in the same scope? Give it a meaningful code name, and use it!

Public Function AdvancedEvaluate(ByVal expression As String) As Variant
    With ScriptingSheet.Range("A1")
        .Formula = expression
        AdvancedEvaluate = .Value
        .ClearContents
    End With
End Function

Now, that's the exact same logic, just more explicit and unnoticeably more efficient in the handling of object references.

To the extent that the idea is to somehow get Excel's calc engine to do the work, other than getting the Excel devs to lift the 255-char limitation in the object model, I think that's as good as it's going to get. It's not a UDF though: in Excel a User-Defined Function refers to a function that can be invoked from a cell - but this function will only ever return an error value to Excel; VBA code can merrily consume it though.


Sample printf implementation Here is my code for printf() implementation in C. Please share your thoughts for improvements.

#define PRINTCHAR(c) putchar(c)
#define MAXLEN 256

// Write a printf function which simulates printf() in C.
int formatAndPrintString(char *pcFmt, va_list lList)
{
    //Holds the number of characters printed, 0 on error
    int nChars = 0;

while(*pcFmt != '\0')
{
    if(*pcFmt == '%')
    {
        pcFmt++;

        int iTemp = 0;
        char acTemp[MAXLEN] = "";
        char *pcTemp = NULL, cTemp = 0;

        switch (*pcFmt)
        {
        case 'd':
            iTemp = va_arg(lList, int);
            itoa(iTemp, acTemp, 10);

            for(int i = 0; i < strlen(acTemp); i++)
            {
                PRINTCHAR(acTemp[i]); nChars++;
            }
            break;
        case 's':
            pcTemp = va_arg(lList, char*);

            while(*pcTemp != '\0')
            {
                PRINTCHAR(*pcTemp); nChars++;
                pcTemp++;
            }
            break;
        case 'c':
            cTemp = va_arg(lList, int);
            PRINTCHAR(cTemp);
            nChars++;
            break;
       default:
          break;
       }
       pcFmt++;
    }
    else
    // print the char to console
    {
       PRINTCHAR(*pcFmt);
       pcFmt++; nChars++;
    }
}
return nChars;
}

// Print function identical to printf() in stdio.h
int printfunc(char *pcFormat, ...)
{
    if(!pcFormat)
    {
        return 0;
    }

    va_list lList;
    va_start(lList, pcFormat);

    return(formatAndPrintString(pcFormat, lList));
}

Bugs

There is no call to va_end()

 The va_start() macro must be called first, and it initializes ap, which can be passed to va_arg() for each argument to be processed.  Calling
 va_end() signals that there are no further arguments, and causes ap to be invalidated.  Note **that each call to va_start() must be matched by a
 call to va_end(), from within the same function**.

Char and Int do not have the same size.

This advice was wrong:

    case 'c':
        cTemp = va_arg(lList, int);

I am surprised this passes your unit tests. It will read a value off the parameter list but also advance it sizeof(int) places. While a character is only of length sizeof(char).

Note  sizeof(int) > sizeof(char)
      sizeof(char) == 1

I am going to leave the above in (because it is a learning experience for me).

From:  @PeteBecker C99 Section:  6.5.2.2/7

  If the expression that denotes the called function has a type that does include a prototype, the arguments are implicitly converted, as if by assignment, to the types of the corresponding parameters, taking the type of each parameter to be the unqualified version of its declared type. The ellipsis notation in a function prototype declarator causes argument type conversion to stop after the last declared parameter. The default argument promotions are performed on trailing arguments.

Thus the char argument is promoted to int when effectively pushed to the stack. So you need to use int when reading it from the var args list. Thus the code is correct.

Unchecked increment:

 default:
          break;
       }
       pcFmt++;

Pretty sure this would break if I used the string "%"

Notes:

Whenever you call PRINTCHAR() you also execute nChars++; so why not put both into the same functioin.

Extensions you should add in the long run.

You cover the bare minimum

You have format d/s/c but in printf there are a lot more with a lot more options: nore Types, type Extension (long/short), width, more formatting, etc.

No Locale Coverage

Output printing is affected a lot by current locale.


PHP creating slugify (clean URL) links in simple way? I think this is the simplest way to slugify urls. You have any contra-indication?

function url_clean($str){
        $str = iconv('utf-8', 'us-ascii//TRANSLIT', $str);
        $clean_str = preg_replace(array('/\'|\"/','/ /'),array('','-'),$str);
        return $clean_str;
}

An alternative, simpler way to code your solution is to use the strtr function which "translates characters". Also I made sure to escape the special characters in the regex.

function url_clean($str) {
    $accent = array(' űáéúőóüöíŰÁÉÚŐÓÜÖÍ');
    $clean  = array('-uaeuoouoiUAEUOOUOI');
    $str = strtr($str, $accent, $clean);
    return preg_replace('/[^A-Za-z0-9\-\.]/', '', $str);
}


Three Cards Monte game I have written the following code for the Three Cards Monte game. It works as expected and returns the required output.

However when I tried to turn it in to the teacher, he keeps saying its too long.

Is there any way to shorten it but make sure it returns the same output?

import java.util.Scanner;

public class ThreeCardsMonte {
    public static void main(String[]args) {
        Scanner keyboard = new Scanner(System.in);
        int ace;
        int guess;

        ace = 1 + (int)(Math.random()*3);

        System.out.println("You slide up to Fast Eddie's card table and plop down your cash.");
        System.out.println("He glances at you out of the corner of his eye and starts shuffling.");
        System.out.println("He lays down three cards.");
        System.out.println("");
        System.out.println("Which one is the ace?");
        System.out.println("");
        System.out.println("##  ##  ##");
        System.out.println("##  ##  ##");
        System.out.println("1   2   3");
        guess = keyboard.nextInt();

        if ((ace == 1) && (guess == 1)) {
            System.out.println("You nailed it! Fast Eddie reluctantly hands over your winnings, scowling.");
            System.out.println("AA  ##  ##");
            System.out.println("AA  ##  ##");
            System.out.println("1   2   3");
  }
        else if ((ace == 1) && (guess != 1)) {
            System.out.println("Ha! Fast Eddie wins again! The ace was card number " +ace+".");
            System.out.println("AA  ##  ##");
            System.out.println("AA  ##  ##");
            System.out.println("1   2   3");
  }
        else if ((ace == 2) && (guess == 2)) {
            System.out.println("You nailed it! Fast Eddie reluctantly hands over your winnings, scowling.");
            System.out.println("##  AA  ##");
            System.out.println("##  AA  ##");
            System.out.println("1   2   3");
  }
        else if ((ace == 2) && (guess != 2)) {
            System.out.println("Ha! Fast Eddie wins again! The ace was card number " +ace+".");
            System.out.println("##  AA  ##");
            System.out.println("##  AA  ##");
            System.out.println("1   2   3");
  }
        else if ((ace == 3) && (guess == 3)) {
            System.out.println("You nailed it! Fast Eddie reluctantly hands over your winnings, scowling.");
            System.out.println("##  ##  AA");
            System.out.println("##  ##  AA");
            System.out.println("1   2   3");
  }
        else if ((ace == 3) && (guess != 3)) {
            System.out.println("Ha! Fast Eddie wins again! The ace was card number " +ace+".");
            System.out.println("##  ##  AA");
            System.out.println("##  ##  AA");
            System.out.println("1   2   3");
          }
     }
}

You have a lot of repeats

 if (ace == guess) {
      System.out.println("You nailed it! Fast Eddie reluctantly hands over your winnings, scowling.");
  }
  else  {
      System.out.println("Ha! Fast Eddie wins again! The ace was card number " +ace+".");
  }

  if (ace == 1) {
      System.out.println("AA  ##  ##");
      System.out.println("AA  ##  ##");
  }
  else if (ace == 2) {
      System.out.println("##  AA  ##");
      System.out.println("##  AA  ##");
  }
  else if (ace == 3) {
      System.out.println("##  ##  AA");
      System.out.println("##  ##  AA");
  }
  System.out.println("1   2   3");

Could put the ace position in an array and just reference it by index
I don't know java to write an array


Object deep diff function using Lodash I'm working on an API testing algorithm within the Postman REST client and I wanted a simple and elegant deep diff function to test my JSON responses. I only have access to Lodash within Postman's testing sandbox.

I couldn't find one on SO or elsewhere online so I decided to write one for myself. My goal was to write the function in less than 10 lines of code so that it is easy to copy/paste and easy to tell what is happening for future people working on the project. I actually got it to work in 5 lines!

_.merge(obj1, obj2, function (objectValue, sourceValue, key, object, source) {
    if ( !(_.isEqual(objectValue, sourceValue)) && (Object(objectValue) !== objectValue)) {
        console.log(key + "\n    Expected: " + sourceValue + "\n    Actual: " + objectValue);
    }
});

Obviously you can change the console.log() to suit your application. I don't actually use object and source but the variables are all there for anyone who wishes to expand this function for their own use. You could create an array of values that are different. Not bad for learning JavaScript 2 weeks ago!

If you don't want to clobber your obj1 you can do a var obj1Copy = _.cloneDeep(obj1) and use your clone object instead.

If you want to trace down through your object from high level to low level, you can remove the && (Object(objectValue) !== objectValue) from the if statement.

Is there any way to improve this code?

Two things could be improved, otherwise it looks good:

_.isEqual alone should be enough for an equality check, i.e. the
check for object equality is implied.
console.log might not be available.


Accept a boolean or a list of acceptable values I want to be able to accept True/False or a list of acceptable exit statuses for a command. This way I can choose whether to check and default to checking exit status 0 unless a list of exit statuses is provided to check against. My first instinct is to do a double reassignment.

def raw(self, ssh_command, check=False):
    # removed ssh calls and processing
    check = [] if not check else check
    check = [0] if check is True else check
    # c.call_result.status -> <encapsulation object>.<named tuple>.<exit status>
    if c.call_result.status not in check:
        raise RuntimeError("Exit status of command {} was {}.".format(ssh_command, c.call_result.status))
    return c.call_result

Is this design simply non-pythonic. Should I refactor to not accept booleans and change check=None to the function with check = [] if check is None?

def raw(self, ssh_command, statuses=None):
    # removed ssh calls and processing
    # This won't work because we'll check against an empty list always even when None is passed.
    statuses = [] if statuses is None else statuses
    if c.call_result.status not in statuses:
        raise RuntimeError("Exit status of command {} was {}.".format(ssh_command, c.call_result.status))
    return c.call_result

EDIT:
Here are what I intended the above to be in order

Accepting booleans or list of exit statuses to accept:

def raw(self, ssh_command, check=False):
    # removed ssh calls and processing which instantiate object c
    check = [0] if check is True else check
    if check and c.call_result.status not in check:
        raise RuntimeError("Exit status of command {} was {}.".format(ssh_command, c.call_result.status))

Not accepting booleans only list of statuses to accept (changed param as this is backwards incompatible):

def raw(self, ssh_command, statuses=None):
    # removed ssh calls and processing which instantiate object c
    if statuses and c.call_result.status not in check:
        raise RuntimeError("Exit status of command {} was {}.".format(ssh_command, c.call_result.status))
    return c.call_result

Of the two I'd go with the second, it doesn't make too much sense to use Booleans and Lists, but Lists and None makes more sense type wise.

The un-Pythonic part of this code, is you assign to check_values twice, which confused me a bit as I thought you were changing check.

I'd also recommend that you try to move away from global variables, c. But otherwise your code is good, :)

If you want to shorten your code a little bit, in the second version you can use or rather than a turnery, this is something that I've noticed to be somewhat popular in JavaScript, but I'm not too sure on how Python folk think of it.

def _raw(self, ssh_command, statuses=None):
    if c.call_result.status not in (statuses or []):
        raise RuntimeError("Exit status of command {} was {}.".format(ssh_command, c.call_result.status))
    return c.call_result


Basic Stack in C++ Using Array I  wrote a stack class in C++ using arrays of fixed width. Could anyone review my code ? I didn't comment on any of the functions, because I thought class itself is self explanatory. Is it a wrong approach or which kind of comments can I write?

#include <stdlib.h>
#include <iostream>

template <class T>
class Stack {
  public:
    Stack(void);    //ctor
    void push(const T& item);
    T pop(void);
    void clear(void);
    T top(void) const;
    bool empty(void) const;
    bool full(void) const;
    void print(void) const;
  private:
    static const int MAX = 50;
    T list[MAX];
    int topPtr;
};

template <class T>
Stack<T>::Stack(void) {
    this->topPtr = -1;
}

template <class T>
void Stack<T>::push(const T& item) {

    if(this->topPtr == this->MAX - 1) {
        std::cerr << "Stack overflow. Can't push" << '\n';
    } else {
        this->topPtr += 1;
        this->list[topPtr] = item;
    }
}

template <class T>
T Stack<T>::pop(void) {
    if(this->topPtr < 0) {
        std::cerr << "Stack is empty. Can't pop" << '\n';
    } else {
        T r_value = this->list[this->topPtr];
        this->topPtr -= 1;
        return r_value;
    }
}

template <class T>
void Stack<T>::clear(void) {
    this->topPtr = -1;
}

template <class T>
T Stack<T>::top(void)const {
    if(this->topPtr < 0) {
        std::cerr << "Stack is empty. No top element." << '\n';
    } else {
        return this->list[topPtr];
    }
}

template <class T>
bool Stack<T>::empty(void) const {
    return (this->topPtr == -1);
}

template <class T>
bool Stack<T>::full(void) const {
    return (this->topPtr == this->MAX -1);
}

template <class T>
void Stack<T>::print(void) const {
    for (int i = this->topPtr; i >= 0; i--)
        std::cout << this->list[i] << '\n';
}

```

Seems like a nice implementation! Here are my thoughts,

this is implicit. Consider removing this, i.e. use topPtr instead of this->topPtr. Similarly, void doesn't need to be used as an argument, consider removing void, i.e. top() instead of top(void).
Consider using std::size_t for MAX. Also, consider naming MAX as MAX_SIZE since it is slightly more descriptive. If using std::size_t, make sure not to subtract one from MAX to avoid overflow. Instead, when checking for equality, add one to the other side.
It might be nicer to use a T* instead of an int for topPtr. nullptr could be used instead of -1 initially for topPtr.
For print consider passing a std::ostream object. This allows for decoupling and one could pass in a std::ostringstream instead of std::cout if desired.
For top and pop, consider using assert instead if/else. An exception could also be used, but I think assert is a little nicer. To include a message, use something similar assert(!empty() && "Stack is empty. Can't pop");. See https://en.cppreference.com/w/cpp/error/assert. Maybe add a comment about the requirements of these functions.
Consider adding documentation for any public function. Include information such as what the function returns, requires, and modifies. For example, top requires there are element in the stack.


groupBy implementation in NodeJS I came across a problem in my NodeJS app, which I solved by creating a method called groupBy. The purpose of this method is simple: take an array, and a function which will select a field from an element in the array. Each element in the array should then be grouped based on this field and returned as an object with the key being the value of the field selected.

var groupBy = function(arr, keySelector) {
    var map = {};
    arr.map(function(element) {
        var key = keySelector(element);
        if(key === undefined)
            throw 'undefined key!';

        map[key] = map[key] || [];
        map[key].push(element);
    });
    return map;
};

It works perfectly, but could it be improved in any way?

Looks OK.

Though I have to ask why you're using map, when you're not really mapping the array to a new array. Using reduce would be more semantically correct, but in this case even just replacing map with forEach would also be more accurate. (The difference is literally only skin-deep; there's zero change in overall functionality.)

I might ask why an undefined value should throw an exception, though. It'll work fine without it, and I might want to group objects based on whether or not they have a certain property or not. Without the exception, I'd be able to get back an object with some of the elements grouped by the key "undefined"

And if I were super nit-picky, I'd say you should always using braces even for one-line "block" like your throw line.

I'm betting, though, that a lot of the time, you'll want to group by a simple, named key, in which case it'd be much easier to pass in a string, rather than a function. A custom function is still very useful, though, so it'd be nice to support both.

I might do:

function groupBy(array, keyOrIterator) {
  var iterator, key;

  // use the function passed in, or create one
  if(typeof key !== 'function') {
    key = String(keyOrIterator);
    iterator = function (item) { return item[key]; };
  } else {
    iterator = keyOrIterator;
  }

  return array.reduce(function (memo, item) {
    var key = iterator(item);
    memo[key] = memo[key] || [];
    memo[key].push(item);
    return memo;
  }, {});
}

Lastly: Libraries like underscore.js and it's "doppelgänger" lo-dash has this and many other helpful functions, ready for you to use. I'm only mentioning it because if you found yourself needed a groupBy function, you'll probably find yourself needing other similar functions, and libraries like those provide that in spades.


Vanilla JavaScript to-do list I'm just learning JavaScript and I started this simple to-do list project to practice on. I got most of the way there, but I couldn't get the clear button to work on any dynamically added inputs. I think this has something to do with where/when the selector variable is defined, but I couldn't work out how to fix it. I'm sure some of the code is not the best way to write it, so any recommendations or fixes would be much appreciated!

var add = document.getElementById('add');
var input = document.getElementsByClassName("input-container");
var remove = document.querySelectorAll('.remove');

var addField = function() {
  var field = document.getElementsByClassName("input-container")[0];
  var clone = field.cloneNode(true);
  var latestInput = document.getElementById("list-container").appendChild(clone);
  latestInput.childNodes[1].value = '';
};

var removeField = function() {
  if (input.length > 1) {
    var parent = this.parentElement.parentElement.parentElement;
    var child = this.parentElement.parentElement;
    parent.removeChild(child);
  }
};

add.addEventListener('click', addField);
remove.forEach(function(field) {
  field.addEventListener('click', removeField);
});
@import url('https://fonts.googleapis.com/css?family=Quicksand:300,400,500,700');
@import url('https://fonts.googleapis.com/css?family=Chewy');
* {
  box-sizing: border-box;
  font-family: 'quicksand', 'sans-serif';
}

html,
body {
  background: #E0E6ED;
  width: 100%;
  height: 100%;
  font-family: 'quicksand', 'sans-serif';
}

h1 {
  font-size: 50px;
  color: #E85D75;
  font-family: 'Chewy', cursive;
}

.main-container h1 {
  margin-bottom: 20px;
}

.text-center {
  text-align: center;
}

.text-input {
  width: 100%;
  border: none;
  border-radius: 4px;
  padding: 10px;
  font-size: 12px;
}

.input-container {
  position: relative;
  margin-top: 8px;
}

.input-container:first-child {
  margin-top: 0;
}

.remove-container {
  position: absolute;
  right: -22px;
  top: 0;
  line-height: 35px;
}

input:focus {
  outline: none;
}

.main-container {
  width: 200px;
  position: relative;
  top: 50%;
  margin-left: auto;
  margin-right: auto;
  transform: perspective(1px) translateY(-50%);
}

.plus-container {
  display: block;
  margin-top: 10px;
}

.fa.fa-plus,
.fa.fa-close {
  color: #C0C8D2;
  cursor: pointer;
}

.fa.fa-plus:hover,
.fa.fa-close:hover {
  color: #E85D75;
}

To-Do List

      <input class="text-input" type="text" placeholder="Type something">

https://codepen.io/mrseanbaines/pen/aLyByJ

Your "clear" button

To get your clear button to work, you just need to add the event listener to each button as its created, rather than at page load.  To do that, take out your .forEach() function and add this line to the end of your addField function:

remove[remove.length - 1].addEventListener('click', removeField);

You'll also want to change the remove declaration so that instead of using querySelectorAll() (not a live list) it uses .getElementsByClassName() (a live list):

var remove = document.getElementsByClassName('remove');

Optimization suggestions

In addField, the field variable is only used once. You can make the code a little more concise by removing that variable and changing your declaration of clone to this:

var clone = document.getElementsByClassName("input-container")[0].cloneNode(true);

You can condense it even further by changing the clone declaration to:

var clone = input[0].cloneNode(true);

In removeField, if you want to reduce the number of variables (and lines) you can replace these lines inside your if statement:

var parent = this.parentElement.parentElement.parentElement;
var child = this.parentElement.parentElement;
parent.removeChild(child);

with these:

var child = this.parentElement.parentElement;
child.parentNode.removeChild(child);

Enhancement suggestion

If you want to add autofocus to each newly-cloned element, so that as it's added it automatically has focus, you could do so easily by adding this line to addField:

latestInput.childNodes[1].focus();

and adding this line to the bottom of your code, to run on load:

input[0].childNodes[1].focus();

Here's what your code looks like with all of the changes mentioned above:

var add = document.getElementById('add');
var input = document.getElementsByClassName("input-container");
var remove = document.getElementsByClassName('remove');

var addField = function() {
  var clone = input[0].cloneNode(true);
  var latestInput = document.getElementById("list-container").appendChild(clone);
  latestInput.childNodes[1].value = '';
  latestInput.childNodes[1].focus();
  remove[remove.length - 1].addEventListener('click', removeField);
};

var removeField = function() {
  if (input.length > 1) {
    var child = this.parentElement.parentElement;
    child.parentNode.removeChild(child);
  }
};

add.addEventListener('click', addField);
remove[0].addEventListener('click', removeField);

input[0].childNodes[1].focus();
@import url('https://fonts.googleapis.com/css?family=Quicksand:300,400,500,700');
@import url('https://fonts.googleapis.com/css?family=Chewy');
* {
  box-sizing: border-box;
  font-family: 'quicksand', 'sans-serif';
}

html,
body {
  background: #E0E6ED;
  width: 100%;
  height: 100%;
  font-family: 'quicksand', 'sans-serif';
}

h1 {
  font-size: 50px;
  color: #E85D75;
  font-family: 'Chewy', cursive;
}

.main-container h1 {
  margin-bottom: 20px;
}

.text-center {
  text-align: center;
}

.text-input {
  width: 100%;
  border: none;
  border-radius: 4px;
  padding: 10px;
  font-size: 12px;
}

.input-container {
  position: relative;
  margin-top: 8px;
}

.input-container:first-child {
  margin-top: 0;
}

.remove-container {
  position: absolute;
  right: -22px;
  top: 0;
  line-height: 35px;
}

input:focus {
  outline: none;
}

.main-container {
  width: 200px;
  position: relative;
  top: 50%;
  margin-left: auto;
  margin-right: auto;
  transform: perspective(1px) translateY(-50%);
}

.plus-container {
  display: block;
  margin-top: 10px;
}

.fa.fa-plus,
.fa.fa-close {
  color: #C0C8D2;
  cursor: pointer;
}

.fa.fa-plus:hover,
.fa.fa-close:hover {
  color: #E85D75;
}

To-Do List

      <input class="text-input" type="text" placeholder="Type something">


Windows function to use NamedTemporaryFile For my package dealing with pptx-files I need to create a temporary *.pptx file, to open in PowerPoint (e.g. to export as PDF). After some research I found that tempfile.NamedTemporaryFile is broken on Windows (you could have a temporary file, or you could get a filename - but you cannot have both). So I tried to build a function, that returns a filename of a temporary file, and takes care of creating and deleting the file internally. After going in circles many times I came up with the following code monster:

import os
from typing import Generator

import tempfile
import pptx

class TempFileGenerator:
    generator = None
    @classmethod
    def get_new_generator(cls, prs):
        TempFileGenerator.generator = cls.temp_generator(prs)
        return cls.generator

    @staticmethod
    def temp_generator(prs: pptx.presentation.Presentation) -> Generator[str, None, None]:
        temp_pptx = None
        try:
            with tempfile.NamedTemporaryFile(suffix='.pptx', delete=False) as temp_pptx:
                temp_pptx.close()  # needed on windows systems to access file
                prs.save(temp_pptx.name)
                yield temp_pptx.name
        finally:
            if temp_pptx is not None:
                try:
                    os.remove(temp_pptx.name)
                except PermissionError as e:  # file still in use somewhere
                    pass
            else:
                print("How did I get here?")

def get_temporary_pptx_filename(prs: pptx.presentation.Presentation) -> str:
    """
    Generates a temporary pptx file. Returns the filename of the temporary file.
    """
    my_generator = TempFileGenerator.get_new_generator(prs)
    for filename in my_generator:
        return filename

Calling get_temporary_pptx_filename() does what I want. Using a class to save the generator ensures, that the clean up in finally does not happen as soon as I return the filename in get_temporary_pptx_filename. But looking at this code monster, I hope there is a better way to do it? If not I guess it would be easier to not use the tempfile module at all.

After some research I found that tempfile.NamedTemporaryFile is broken on Windows (you could have a temporary file, or you could get a filename - but you cannot have both).

This is just flat out wrong. Maybe you're misunderstanding something and conflating two things. But this is just factually incorrect. And can be seen in your code:

temp_pptx.close()  # needed on windows systems to access file
prs.save(temp_pptx.name)

The first line is closing the temporary file, and the second is getting it's filename.

  needed on windows systems to access file

This is due to a security feature.

  The file is created securely, using the same rules as mkstemp().

  Creates a temporary file in the most secure manner possible. There are no race conditions in the file’s creation, assuming that the platform properly implements the os.O_EXCL flag for os.open(). The file is readable and writable only by the creating user ID. If the platform uses permission bits to indicate whether a file is executable, the file is executable by no one. The file descriptor is not inherited by child processes.

Clearly you don't need this, so tempfile is pretty much useless to you and you will be fighting it.
Your code looks very much like a mix of an exploit of how generators and garbage collection work, and some dodgy monkey patch to get around tempfile. Both are on a level of wtf/m that I'd suggest just writing your own temporary file class.

import os

class TemporaryFile:
    __slots__ = ('_file',)

    def __init__(self, file):
        self._file = file

    @classmethod
    def open(cls, *args, **kwargs):
        return cls(open(*args, **kwargs))

    def __enter__(self):
        return self._file.__enter__()

    def __exit__(self, exc_type, exc_value, traceback):
        ret = self._file.__exit__(exc_type, exc_value, traceback)
        os.remove(self._file.name)
        return ret

if __name__ == '__main__':
    with TemporaryFile.open('foo.txt', 'w+') as f:
        pass
    with TemporaryFile(open('foo.txt', 'w+')) as f:
        pass

I can write to the file using Atom using the above. Even if Excel has more security operations, then you can easily work around them without monkey patches.


Scan array for 3 consecutive numbers that sum to 7 Write a function lucky_sevens?(numbers), which takes in an array of
  integers and returns true if any three consecutive elements sum to 7.

lucky_sevens?([2,1,5,1,0]) == true # => 1 + 5 + 1 == 7
lucky_sevens?([0,-2,1,8]) == true # => -2 + 1 + 8 == 7
lucky_sevens?([7,7,7,7]) == false
lucky_sevens?([3,4,3,4]) == false

  Make sure your code correctly checks for edge cases (i.e. the first
  and last elements of the array).

While my code does seem to work (I did some test cases), I know it can be improved. There's no reason to have double puts and return statements. I'm also wondering if there are general improvements to the syntax itself that can make it much more readable.

def lucky_sevens?(numbers)
  i = 0
  while i < numbers.length - 2

    if numbers[i] + numbers[i + 1] + numbers[i + 2] == 7
        puts true
        return true
    end
    i+=1
    end
    puts false
    return false
end

Your code, is way too imperative to be idiomatic Ruby.

The only reasonable improvement is re-writing it from scratch:

def lucky_sevens?(numbers)
  groups_of_three(numbers)
    .any? {|group| group.inject(:+) == 7}
end

Knowing that inject(:+) means sum, this reads like English. Implementing groups_of_three is left as an exercise for the reader.


jQuery wait for all paginations I want to download data for all of my repos from the Github API.

Github paginates their responses and includes a header Link type:"next" when there's another page available. So I know I want to wait for all the pages to be received... but I don't know initially how many pages there are going to be, and I don't know if there's a next page until the one before it comes in.

So far I've got this, and it works:

var allRepos = [];
var repoPageReqs = [$.Deferred()]; // <-- sketchy
function receivedReposPage(data, status, req) {

    //handle error inadequately but whatever for now
    if (status !== "success") { console.log(data, status, req); return; }

    console.log("Received repos from Github:", data);
    for (i in data) {
        allRepos.push(data[i]);
    }
    saveRepos(allRepos);

    var headerLink = req.getResponseHeader("Link");
    var headerLinkNext = parseLinkHeader(headerLink)["next"];
    if (headerLinkNext) {
        console.log("Following 'next' header link, collecting paginated repos.");
        repoPageReqs.push(downloadRepos(headerLinkNext));
    } else {
        repoPageReqs[0].resolve(); // <-- resolve sketchiness
    }
}

function downloadRepos(path) {
    return $.get(path, receivedReposPage);
}

function downloadAllRepos(path) {
    repoPageReqs.push(downloadRepos(path));
    return $.when.apply(null, repoPageReqs);
}

$(function () {
    downloadAllRepos("https://api.github.com/my/repos/path")
      .then(function () {
           console.log("finished downloading all repos");
      })
});

The reason I use the sketchy [$.Deferred()] was because the $.when would resolve before the callback pushed the new $.get promise into the promisey array.

Is there a better way to make/resolve this kind of "unraveling" chain?

Using $.when is useful when you know you have N asynchronous operations that have to resolve. However, in this case you, as you say, don't know how many there'll be, so the simpler thing would be to create one "master" promise/Deferred and resolve that at the appropriate time.

Which is basically what you're already doing with that repoPageReqs[0] promise, you just have it mixed up in an array and insisting on all this $.when stuff that's not useful here.

But first, some small stuff:

You have i as an implied global variable in your for(i in data) - that's a no-no.
I'm also wary of the parseLinkHeader(headerLink)["next"] line. For one, it might as well be .next instead of the bracket notation, but more importantly, is it safe to assume that parseLinkHeader will always return an object? If it fails, I'd rather have it return null or undefined, but then the code will fail when trying something like null["next"].

I'd don't know how your parseLinkHeader function works, but it just smells a little strange to me.

Anyway, point is to use a single "overall" promise, instead of all that $.when stuff. Something like:

function downloadAllRepos(path) {
  var deferred = $.Deferred(), // the "master" promise
      repos = [];

  function parseLinkHeader(header) { ... }

  function receivedReposPage(data, status, req) {
    if (status !== 'success') {
      deferred.reject(data, status, req); // reject the promise if any page fails to load
      return;
    }

    // if data is an array, then you should *not* use a for..in loop
    // but a regular for loop instead.  for..in is made to iterate
    // object properties, not array elements. Calling the counter var
    // i makes me think you're dealing with an array... but I don't
    // know what the data variable is, so I've kept the for..in loop
    // and the assumption that it's an object rather than an array,
    // but added a sanity check to skip over any inherited properties
    // that might otherwise get erroneously iterated.
    // (And if data is an array, you might get away with simply
    // saying `repos = repos.concat(data)` instead of looping!)
    for (var prop in data) {
      if (!data.hasOwnProperty(prop)) continue;
      allRepos.push(data[prop]);
    }

    // you may want to call deferred.notify() here to let
    // others know that you've downloaded another page, e.g.
    // you could send the number of repos gotten so far:
    deferred.notify(repos.length);

    // a slightly safer approach to getting/parsing the header
    var linkHeader = parseLinkHeader(req.getResponseHeader('Link'));
    if (linkHeader && linkHeader.next) {
      getPage(linkHeader.next); // get the next page
    } else {
      deferred.resolve(repos); // all done, resolve the promise
    }
  }

  function getPage(path) {
    $.get(path, receivedReposPage);
  }

  // get the first page
  getPage(path);

  // return a promise
  return deferred.promise();
}

// Example
downloadAllRepos("https://api.github.com/my/repos/path")
  .progress(function (repoCount) {
    console.log("Downloaded " + repoCount + " repo(s) so far...");
  })
  .success(function (repos) {
    // code to run when *all* pages have loaded
  })
  .failure(function (data, status, req) {
    // code to run if a page failed to load
    console.log(data, status, req);
  });

I've placed everything inside the downloadAllRepos() function to reduce the overall footprint.


C# Tasks for Asynchronous Operations I currently use the following code to asynchronously create a cimsession to a remote host, query the remote host, return multiple values and finally updated some textboxes on my WPF window. Just wanting to know if anyone can see any glaring issues (I'm new to all things async and c# :) ) or a better was of doing it:

using System.Threading;
using System.Threading.Tasks;
using Microsoft.Management.Infrastructure;

//creating cimsession to remote host
CimSession session = await Task.Run(() =>
{
     return CimSession.Create(computerHostName);
});

//getting username, bootupstate, manufacturer and model
var GI1 = await Task.Run(() =>
{
    var results = new List<string>();
    IEnumerable<CimInstance> queryResults = session.QueryInstances(nameSpace, WQL, "SELECT Username, BootUpState, Manufacturer, Model FROM Win32_ComputerSystem");
    foreach(CimInstance i in queryResults)
    {
        results.Add(i.CimInstanceProperties["Username"].Value.ToString());
        results.Add(i.CimInstanceProperties["BootUpState"].Value.ToString());
        results.Add(i.CimInstanceProperties["Manufacturer"].Value.ToString());
        results.Add(i.CimInstanceProperties["Model"].Value.ToString());
    }
    //returning all the variables
    return results;
});
//adding username, bootupstate, manufacturer and model to their textboxes
GIUS.Text = GI1[0];
GIBS.Text = GI1[1];
GIMF.Text = GI1[2];
GIMD.Text = GI1[3];

Any suggestions or help is appreciated :)

//creating cimsession to remote host
CimSession session = await Task.Run(() =>
{
     return CimSession.Create(computerHostName);
});

Task.Run is a red flag. This should be

CimSession session = await CimSession.CreateAsync(computerHostName);

Similarly for every method you call on session which has a ...Async version, you should use the ...Async version.

It seems that you're calling this from a GUI thread. That means you need to be careful about where your continuations are executed. I would suggest pulling the async calls out into a method and using .ConfigureAwait(false) on all of them, and then calling that method from the GUI thread with ContinueWith(...) to update the GUI, but since you refuse to provide more context I can't be more specific.


Finding large Fibonacci Number in Python I'm doing Project Euler at 1000-digit Fibonacci number - Problem 25 which says

  The Fibonacci sequence is defined by the recurrence relation:

  Fn = Fn−1 + Fn−2, where F1 = 1 and F2 = 1.

  Hence the first 12 terms will be:

F1 = 1
F2 = 1
F3 = 2
F4 = 3
F5 = 5
F6 = 8
F7 = 13
F8 = 21
F9 = 34
F10 = 55
F11 = 89
F12 = 144

  The 12th term, F12, is the first term to contain three digits.

  What is the index of the first term in the Fibonacci sequence to contain 1000 digits?

I approached this by writing a recursive function in Python that finds the nth Fibonacci number as follows:

def Fibonacci(n):
    if n == 1:
        return 1
    elif n == 2:
        return 1
    else:
        return (Fibonacci(n-1) + Fibonacci(n-2))

However, this function runs very, very slowly. It slows down severely as n approaches 100.

What can I do to make it faster?

Issue
Your issue is you compute fibonacci(i) for one number multiple times.
To understand this, consider computing fibonacci(5). The function would call fibonacci(4) and fibonacci(3) at the end. Now what happens in fibonacci(4)? It would call fibonacci(3) and fibonacci(2). Now you can notice, that the function gets called 2 times for the number 3, which is a serious issue, since that can go VERY deep in the recursion and you can gain a massive overhead because of this.
Solution
Because fibonacci is a pure function, it returns the same value every time it is called with the same parameters. So there's a simple trick you can do to avoid so much time on recalculating what you already found. You can store the result of fibonacci for a specific number in an array, and if it's called again, just return that instead of computing everything again. This trick is called memoization. The Fibonacci sequence is often used as the introductory example for dynamic programming and memoization.
See this nice video explaining the concept of memoization and dynamic programming
Code
This is a simple implementation of memoization for this problem.
cache = {}

def fibonacci(n):
    if n in cache:
        return cache[n]

    if n == 1 or n == 2:
        return 1
    else:
        result = fibonacci(n-1) + fibonacci(n-2)
        cache[n] = result
        return result


Simple beginner Java calculator This is my first calculator in Java. I just started learning a week ago so any feedback (positive and negative) is welcome. Tell me what's good and what I can improve on.

import java.util.Scanner;

public class Calculator {

    public static void main(String[] args) {

        // Creates scanner object
        Scanner scanner = new Scanner(System.in);

        // Controls the start and stop of the program
        boolean run = true;

        // Sets and resets the operand and answer variables
        int fnum = 0;
        int snum = 0;
        int answer = 0;

        // While loop that keeps the program running until user exits
        while (run) {

            // Calculator interface
            System.out.println("=====================================");
            System.out.println("           Calculator V1.0         ");
            System.out.println("=====================================");
            System.out.println("1. Addition");
            System.out.println("2. Subtraction");
            System.out.println("3. Multiplication");
            System.out.println("4. Division");
            System.out.println("5. Exit");
            System.out.printf("Please pick an operation(1-5): ");

            // Prompts user to choose an operation
            int operation = scanner.nextInt();

            // Does stuff depending on what integer the user enters
            switch (operation) {
            case 1:
                System.out.printf("Enter first number: ");
                fnum = scanner.nextInt();
                System.out.printf("Enter second number: ");
                snum = scanner.nextInt();
                answer = fnum + snum;
                System.out.println("The answer is: " + answer);
                break;

            case 2:
                System.out.printf("Enter first number: ");
                fnum = scanner.nextInt();
                System.out.printf("Enter second number: ");
                snum = scanner.nextInt();
                answer = fnum - snum;
                System.out.println("The answer is: " + answer);
                break;

            case 3:
                System.out.printf("Enter first number: ");
                fnum= scanner.nextInt();
                System.out.printf("Enter second number: ");
                snum = scanner.nextInt();
                answer = fnum * snum;
                System.out.println("The answer is: " + answer);
                break;

            case 4:
                System.out.printf("Enter first number: ");
                fnum = scanner.nextInt();
                System.out.printf("Enter second number: ");
                snum = scanner.nextInt();
                answer = fnum / snum;
                System.out.println("The answer is: " + answer);
                break;

            case 5:
                System.out.println("Thank you for using the calculator!");
                scanner.close();
                run = false;
                break;

            default:
                System.out.println("Invalid operation!");
                break;
            }
        }
    }
}

Comments

// Does stuff depending on what integer the user enters

That is not a good comment.  A comment should say why the code does something if it is not clear and the code should say what it does (if the code is unclear, you can use a comment, but this is also a signal you should write clearer code), this comment is just plain noise.

If you are using the comment to remember what the code does, not just because some teacher/book said to, a comment here is perfectly fine:

// Choose action based on input

Duplicated code

case 1:
    System.out.printf("Enter first number: ");
    fnum = scanner.nextInt();
    System.out.printf("Enter second number: ");
    snum = scanner.nextInt();
    answer = fnum + snum;
    System.out.println("The answer is: " + answer);
    break;

case 2:
    System.out.printf("Enter first number: ");
    fnum = scanner.nextInt();
    System.out.printf("Enter second number: ");
    snum = scanner.nextInt();
    answer = fnum - snum;
    System.out.println("The answer is: " + answer);
    break;

Anything catch your eye there?  Lots and lots of copy/pasted code.  Most of that can be moved to before or after the switch:

System.out.printf("Enter first number: ");
fnum = scanner.nextInt();
System.out.printf("Enter second number: ");
snum = scanner.nextInt();

switch (operation) {
case 1:
    answer = fnum + snum;
    break;

case 2:
    answer = fnum - snum;
    break;
}

System.out.println("The answer is: " + answer);

Output

This really is a nitpick for now, but this:

System.out.println("           Calculator V1.0         ");

Will display the same as this:

System.out.println("           Calculator V1.0");

Methods

This is a pretty straightforward program, but you should start separating unique components of your program and creating more methods.  For example, you could create a method for input, a method for calculating the answer, and a method for output.

Enums

Another thing you should look into is using an enum to designate the selected operation.  Using case Operation.Addition: is clearer than case 1:, and there is a very limited set of data that allowed, which is demonstrated and enforced by the enum, which only accepts the allowed set, and is not enforced by an int.

Bug

Your division operation has a bug:

snum = scanner.nextInt();
answer = fnum / snum;

What happens when the user enters 0?  Your program will crash.  You must always test for division by 0 in cases like this:

snum = scanner.nextInt();
if (snum == 0) {
    System.out.println("Invalid input - cannot divide by 0.");
    break;
}
answer = fnum / snum;


NumPy eliminate double loop How can I vectorize this code snippet and eliminate this double loop? Even with "only" 1001 elements in a this takes almost 30s.

a = np.linspace(-.5, .5, 1001)
S = np.zeros((a.size, a.size))

# east, north and tp are np.arrays of equal length
for l, sy in enumerate(a):
    for k, sx in enumerate(a):
            S[l,k] = np.sum((east*sx + north*sy - tp)**2)

This should do it:

S = np.sum((east*a[:, None, None] + north*a[:, None] - tp)**2, axis=-1)

But if the east north and tp arrays are large it may actually perform worse, as it avoids looping but creates an intermediate array of a.size**2 * east.size items.

If you expand your expression, you could also do:

e2 = np.dot(east, east)
n2 = np.dot(north, north)
t2 = np.dot(tp, tp)
en = np.dot(east, north)
et = np.dot(east, tp)
nt = np.dot(north, tp)

a2 = a*a

S = (e2*a2[:, None, None] + b2*a2[:, None] + t2 +
     2*en*a[:, None, None]*a[:, None] - 2*et*a[:, None, None] -
     2*nt*a[:, None])

which should give the same result without the ginormous intermediate array.


Timetable viewer - Python 3.4 My friend and I, have made a timetable viewer in Python 3.4 for a mock timetable. The format is a 2 week school timetable with 5 daily lessons with each lesson having a room. We have tried to make it as object-oriented as possible, yet is the solution overkill? Our goal was to make the classes into a module so you can just import timetable and create your own Lesson() and Day() objects to make your own timetable. So, is our solution overkill, and any general feedback on the code?

class Lesson:
    def __init__(self, name, room):
        self.room = room
        self.name = name
    def __repr__(self):
        return ('{}: {}').format(self.name, self.room)

class Day:
    def __init__(self, name, week, lessons):
        self.name = name
        self.week = week
        self.lessons = lessons
    def __repr__(self):
        return ((str([x for x in self.lessons]).replace('[', ' ')).replace(']', '')).replace(',', '\n')

physics = Lesson("Physics", "B27")
prod_des = Lesson("Product Design", "B2")
comp_sci = Lesson("Computer Science", "B14")
french = Lesson("French", "C9")
english = Lesson("English", "C11")
maths = Lesson("Maths", "T15")
private_study = Lesson("Private Study", "N/A")
chem = Lesson("Chemistry", "B12")
engineering = Lesson("Engineering", "Off Campus")
biol = Lesson("Biology", "B24")
add_math = Lesson("Additional Math", "T19")
games = Lesson("Games", "Sports Field")
re = Lesson("R.E.", "M20")
engineering_voc = Lesson("Engineering (Vocational)", "B2")

monday = Day("Monday", "Week 1",[physics, prod_des, comp_sci, french, english])
tuesday = Day("Tuesday", "Week 1", [english, maths, prod_des, physics, private_study])
wednesday = Day("Wednesday", "Week 1", [french, chem, comp_sci, engineering, engineering])
thursday = Day("Thursday", "Week 1", [french, private_study, biol, maths, english])
friday = Day("Friday", "Week 1", [comp_sci, prod_des, maths, add_math, games])

monday2 = Day("Monday", "Week 2",[maths, prod_des, biol, english, private_study])
tuesday2 = Day("Tuesday", "Week 2", [prod_des, french, chem, maths, private_study])
wednesday2 = Day("Wednesday", "Week 2", [comp_sci, physics, re, engineering, engineering])
thursday2 = Day("Thursday", "Week 2", [games, maths, biol, french, english])
friday2 = Day("Friday", "Week 2", [french, chem, engineering_voc, english, comp_sci])

days = { 1: monday, 2: tuesday, 3: wednesday, 4: thursday, 5: friday,
         6: monday2, 7: tuesday2, 8: wednesday2, 9: thursday2, 10: friday2
}

while True:
    while True:
        try:
            day = int(input("\nDay: "))
            if day in days.keys():
                day_obj = days[day]
                print(day_obj.name, day_obj.week)
                print(day_obj)
                break
            raise ValueError
        except:
            print("It has to be a number between 1 and 10")
    while True:
        again = input("Again? (y/n): ").lower()
        if again == "n":
            quit()
        elif again not in ["y", "n"]:
            print("Type 'y' or 'n'")
        elif again == "y":
            break

Possible improvements:

Don't use __repr__ for a human readable version of the object. That is what __str__ is for. When writing your own __rerp__ you should aim to make eval(repr(obj)) work.
Rather than str.replace, you could str.format the values to the correct format. And so I'd change your Day.__repr__ to something more like:

def __str__(self):
    return '\n'.join(' {!s}'.format(x) for x in self.lessons)

Rather than making two near duplicate loops, you could abstract the logic into one function, get_input. You could also make it so that it shows the user what input they are allowed to pick.

This could be implemented as:

def get_input(question, valid):
    question += '[{}] '.format('/'.join(valid))
    valid = set(valid)
    while True:
        answer = input(question).lower()
        if answer in valid:
            return answer

I'd change Day.__str__ so that it also displays the name and week.
I'd rather read a JSON file, than build the week structure in your program. This is as you may want to later build a GUI, and allow other students to enter their time-tables.

I'll leave this down to the both of you to look into. :)

Excluding the last bullet point I'd change your code to:

class Lesson:
    def __init__(self, name, room):
        self.room = room
        self.name = name

    def __str__(self):
        return '{}: {}'.format(self.name, self.room)

class Day:
    def __init__(self, name, week, lessons):
        self.name = name
        self.week = week
        self.lessons = lessons

    def __str__(self):
        return '{} {}\n{}'.format(
            self.name,
            self.week,
            '\n'.join(' {!s}'.format(x) for x in self.lessons)
        )

def get_input(question, valid):
    question += '[{}] '.format('/'.join(valid))
    valid = set(valid)
    while True:
        answer = input(question).lower()
        if answer in valid:
            return answer

days_str = [str(day) for day in sorted(days.keys())]
while True:
    day = get_input('Day: ', days_str)
    print(days[int(day)])
    if 'n' == get_input('Again? ', ['y', 'n']):
        break


Monkey testing a SmartCard library My definition of monkey testing is basically playing with a program in as if I was a monkey (press every button, unplug things, go in the wrong order..etc etc)

So I made a rather simple SmartCard library that when I "enable" it it just polls the smart card reader for a card, and reports back information. see code

private void CardStatusChange()
{
    readerState[0].dwCurrentState = SCardState.SCARD_STATE_UNAWARE;
    while (run)
    {
        System.Threading.Thread.Sleep(100);
        if (SCWrapper.SCardGetStatusChange(scHandle, UInt32.MaxValue, readerState, 1) == SCardFunctionReturnCodes.SCARD_S_SUCCESS)
        {
            readerState[0].dwCurrentState = readerState[0].dwEventState;
            SendCardStatus(readerState[0].dwEventState);
        }
    }
}

private void SendCardStatus(SCardState sCardState)
{
        //case SCardState.SCARD_STATE_UNAWARE:
        //case SCardState.SCARD_STATE_IGNORE:
        //case SCardState.SCARD_STATE_CHANGED:
        //case SCardState.SCARD_STATE_UNKNOWN:
        //case SCardState.SCARD_STATE_UNAVAILABLE:
        //case SCardState.SCARD_STATE_EMPTY:
    if (HasStateMask(sCardState, SCardState.SCARD_STATE_EMPTY))
        OnCardUnplugged(); //fire event
        //case SCardState.SCARD_STATE_PRESENT:
    if (HasStateMask(sCardState, SCardState.SCARD_STATE_PRESENT))
    {
        OnCardPluggedIn(); //fire event
        SendCardATR(readerState[0].rgbAtr); //fire event
    }
        //case SCardState.SCARD_STATE_ATRMATCH:
    OnATRMatch(HasStateMask(sCardState, SCardState.SCARD_STATE_ATRMATCH));
        //case SCardState.SCARD_STATE_EXCLUSIVE:
        //case SCardState.SCARD_STATE_INUSE:
    OnCardInUse(HasStateMask(sCardState, SCardState.SCARD_STATE_INUSE));
        //case SCardState.SCARD_STATE_MUTE:
        //case SCardState.SCARD_STATE_UNPOWERED:
}

private bool HasStateMask(SCardState theValue, SCardState theMask)
{
    return ((theValue & theMask) == theMask);
}

This part works as expected in that while my thread is running I can plug in and unplug a smartcard as often and as quickly as I want without problem. What I'm struggling with is the Threading portion of it. Visual Studio screams at me for using suspend and I found a bug when using suspend in my thread so I am taking that out (in other words don't comment on that) but this is how I "open" and "enable" my smart card

public override bool Open()
{
    readerThread = new Thread(CardStatusChange);
    //code here to setup the smart card
    return true;
}

public override bool Enable()
{
    run = true;
    //thread start checking for card events
    switch (readerThread.ThreadState)
    {//this is probably a mistake
        case ThreadState.AbortRequested:
            break;
        case ThreadState.Aborted:
            break;
        case ThreadState.Background:
            break;
        case ThreadState.Running:
            break;
        case ThreadState.StopRequested:
            break;
        case ThreadState.Stopped:
            break;
        case ThreadState.SuspendRequested:
            break;
        case ThreadState.Suspended:
            readerThread.Resume();//TODO remove

            break;
        case ThreadState.Unstarted:
            readerThread.Start();

            break;
        case ThreadState.WaitSleepJoin:
            break;
        default:
            break;
    }

    return true;
}
public override void Disable()
{
    run = false;
    readerThread.Suspend(); //TODO remove
}

I've never been too good at threading, but I do need this as a thread (I think I do atleast I am open to other ideas). So how should I really Enable and Disable? I used to just set run to true or false, but my monkey testing showed that this doesn't always work and sometimes Windows would cry that it can't start a running thread... SO I dunno what to do. Any thoughts?

As requested by Ryan Gates I should post my results. I don't remember the entire scope of what i did, but I do know where the code is :)

So the basics are that I want the thread to be able to exit without causing any strange hicups if the user decides to use my library incorrectly. So I made the thread turn into a background thread. I was able to remove the ugly switch statement and just check the appropriate bits are set to make sure I can start my thread (basically only allows one thread to read a smart card instead of multiple reading the same card)

On Disable what I did was set my flag to kill the loop in my readerThread thread. I then wait for it to exit by using the thread.Join Clear as mud??? I can try to clear up more if anyone has questions.

public override bool Open()
{
    //Code to setup SmartCard

    readerThread = new Thread(CardStatusChange);
    readerThread.IsBackground = true;

    return true;
}
public override bool Enable()
{
    run = true;
    //thread start checking for card events
    Globals.tracer.TraceInformation("Checking Thread State:{0}", readerThread.ThreadState);
    var startable = (readerThread.ThreadState & ThreadState.Unstarted);
    if (startable > 0)
        readerThread.Start();

    return true;
}
public override void Disable()
{
    run = false;
    if(readerThread.ThreadState == ThreadState.Unstarted)
        readerThread.Join();
}


How can I display the results of mysql_fetch_assoc in a dynamic table? I have a MySQL database with several columns and two rows of data, so far. I'm using mysql_fetch_assoc to return the data in database, but I don't know how to put it all together in a table so that I can display it in another file.

Here is the code of the file that does the process (query.php):

<?php

// Configure connection settings

$db = 'scorecard';
$db_admin = 'root';
$db_password = '********';
$tablename = 'scoreboard';

// Title

//echo "DIV!";

// Connect to DB

$sql = mysql_connect("localhost", $db_admin, $db_password)
or die(mysql_error());

mysql_select_db("$db", $sql);

// Fetch the data

$query = "SELECT * FROM $tablename";
$result = mysql_query($query);
echo mysql_error();

echo "
\n";
echo "
\n";

// Return the results, loop through them and echo

while($row = mysql_fetch_array($result, MYSQL_ASSOC))
{
$codcliente = $row['codcliente'];
$nombre = $row['nombre'];
$ejecutivo = $row['ejecutivo'];
$banca_as400 = $row['banca_as400'];
$banca_real = $row['banca_real'];
$ingresos = $row['ingresos'];
$ciiu = $row['ciiu'];
$division = $row['division'];
$actividad = $row['actividad'];
$riesgo_industria = $row['riesgo_industria'];
$riesgo_cliente = $row['riesgo_cliente'];
$fecha = $row['fecha'];
$analista = $row['analista'];
echo "
$codcliente	$nombre	$ejecutivo	$banca_as400	$banca_real	$ingresos	$ciiu	$division	$actividad	$riesgo_industria	$riesgo_cliente	$fecha	$analista
\n";
echo "
$codcliente	$nombre	$ejecutivo	$banca_as400	$banca_real	$ingresos	$ciiu	$division	$actividad	$riesgo_industria	$riesgo_cliente	$fecha	$analista
\n";
echo "
\n";
}

And this is the result from the page that displays the process (index.php):

CAN'T DISPLAY SINCE I DON'T HAVE ENOUGH REPUTATION POINTS :(

As you can see, I have a table already in place and I would like the results to fall below as additional rows, but they come out all squashed together. I've tried several mysql_fetch types, but nothing works. The query results are displayed in a JavaScript DIV that is automatically refreshed by an AJAX script from another file (ajax.js). If you need to see the code for index.php or ajax.js, I could provide it.

Thanks!

Aren't you forgetting something, called ?

Try

<?php

// Configure connection settings

$db = 'scorecard';
$db_admin = 'root';
$db_password = '********';
$tablename = 'scoreboard';

// Title

//echo "DIV!";

// Connect to DB

$sql = mysql_connect("localhost", $db_admin, $db_password)
or die(mysql_error());

mysql_select_db("$db", $sql);

// Fetch the data

$query = "SELECT * FROM $tablename";
$result = mysql_query($query);
echo mysql_error();

echo "
\n";

// Return the results, loop through them and echo

while($row = mysql_fetch_assoc($result))
{
    echo '
'."\n";
    echo "{$row['codcliente']}	\n" . "{$row['nombre']}	\n" . "{$row['ejecutivo']}	\n" . "{$row['banca_as400']}	\n" . "{$row['banca_real']}	\n" . "{$row['ingresos']}	\n" . "{$row['ciiu']}	\n" . "{$row['division']}	\n" . "{$row['actividad']}	\n" . "{$row['riesgo_industria']}	\n" . "{$row['riesgo_cliente']}	\n" . "{$row['fecha']}	\n" . "{$row['analista']}	\n";

    echo '
'."\n";
}

echo "
\n";

?>


Low-latency implementation of converting non null terminated char* to unsigned int How would you rewrite this code which I have now:

uint32_t Utils::stringToInt(unsigned char const* buffer, int len)
{
    uint32_t result = buffer[0] - '0';
    for (int i = 1; i < len; i++) {
        result *= 10;
        result += buffer[i] - '0';
    }
    return result;
}

I need the lowest possible latency. It's very simple, and we don't need to care about '-', so I expect some faster implementation is possible.

Be careful, this code assumes that there is at least one digit in the buffer. To make it work with empty buffer too:

uint32_t result = 0;
for (int i = 0; i < len; i++) {
    result *= 10;
    result += buffer[i] - '0';
}
return result;

In terms of performance, I don't think it's possible to achieve better using clean techniques. There are some optimizations that might help performance, but hurt readability and maintainability. For example you could try loop unrolling:

if (len == 0) return 0;

if (len == 1) {
    return buffer[0] - '0';
}

if (len == 2) {
    return (buffer[0] - '0') * 10 + buffer[1] - '0';
}

// ... and so on, until the max supported length

Yes it's ugly. But loop unrolling tends to improve performance. Tends, the result may depend on your compiler, platform, or the weather. You'll have to measure performance before and after to see if this optimization actually helps our not.


Nested if statements with 3 different parameters I have a block of code below. The allDone() method at the bottom should only be run if the allCompleted == true. It should run through each of the statements to test.

allCompleted: This starts as true so the below logic works right.
run*.Checked: This is based on a check box in a form. This block should only run if this box is checked.
cmd: This is a generic string variable stating whether another part of the code (not shown here) was run successfully. If it has run successfully this string will read "done".

After those options, if all enabled (run*.Checked == true) methods have returned the cmd* string as "done" (everything that's checked has run successfully) then allCompleted should be true at the end so allDone() gets run.

If one single enabled method returns false (there was an error somewhere or otherwise it did not return "done"), then the allDone() method should not be run and the code will continue, skipping the last if (allCompleted) statement.

bool allCompleted = true;

if (runPart1.Checked)
    if (cmdPart1 == "done")
        allCompleted = ((allCompleted)? true : false);
    else
        allCompleted = false;

if (runPart2.Checked)
    if (cmdPart2 == "done")
        allCompleted = ((allCompleted) ? true : false);
    else
        allCompleted = false;

if (runPart3.Checked)
    if (cmdPart3 == "done")
        allCompleted = ((allCompleted) ? true : false);
    else
        allCompleted = false;

if (runPart4.Checked)
    if (cmdPart4 == "done")
        allCompleted = ((allCompleted) ? true : false);
    else
        allCompleted = false;

if (allCompleted)
    allDone();

So if at anytime one of the enabled parts fail the code will basically just move on.

As it stands this code works, I just feel like it could be written better. Is this the best way or have I got it? Something about it makes me feel awkward still.

EDIT: Also, each time one of the parts completes, it runs this method, so it will run a few times being false in the end until the last one runs and all the others are "done" in which case it should completes and run allDone().

Others are giving you refactoring ideas, so I will just focus on one statement in your original code that is repeated 4 times.

allCompleted = ((allCompleted) ? true : false);

Look at this. You are inspecting allCompleted. If the value is true,  you're setting it to true. If it is not true, you're setting it to false. You are setting it to what it already is in a sort of non-intuitive way. You could very well rewrite it as the below and have the exact same meaning.

allCompleted = allCompleted ? allCompleted : allCompleted;

Simplify that to

allCompleted = allCompleted;

And then simplify that to leaving it out altogether.

if(runPart1.Checked)
   if (cmdPart1 != "done")
      allCompleted = false;

Code can be complicated enough as it is. Try not to add further complexity by including code that can be non-obvious in the fact that it does nothing at all!


Power function in JavaScript I know that there may be better ways of writing this do you think that this code is written well enough to be used in a real world application?

    var pow = function ( base, power ) {
    var result = 1;
    if ( power < 0 ) {
        return ( 1 / pow( base, -(power)) );
    }
    for ( var i = 1; i <= power; i++ ) {
        result = result * base;
    }
    return result;
};

For a power function with an integer exponent you can loop through the bits in the exponent instead of making a linear loop:

function pow(base, power) {
    if (power < 0) return 1 / pow(base, -power);
    var b = base, result = 1;
    while (power > 0) {
        if ((power & 1) != 0) {
            result *= b;
        }
        power >>= 1;
        b *= b;
    }
    return result;
}

This will only loop as many times as there are bits used in the exponent, for a value like 1000 that means 10 iterations instead of 1000.

Testing this with pow(2, 1000) in Firefox shows that it is about 70 times faster (but the built in method is still even 8 times faster).


Should I design my enumeration in some way that indicates what the highest value is? This is a subtle question about design in which I want to find the most elegant and appropriate solution.

I have an enumeration representing a French card deck (see code below). With it I need to do certain things, such as displaying the elements in a table. I'll want to display the elements starting with ACE (the highest) down to deuce. The problem is that I don't want my UI part of the code to know which one is the highest and use a descending iterator. Should I have a method in my enumeration, something like "EnumSet descending()" ? I don't want to initialise the constants with integers that indicates their values, since in poker the cards don't have a numerical value per se, just an order. Probably it's just fine if users of this enum are supposed to know that an ACE has the highest value and display values accordingly to what they want, but just wondering if there is a better solution.

public enum Rank {
    DEUCE ("2"),
    THREE ("3"),
    FOUR ("4"),
    FIVE ("5"),
    SIX ("6"),
    SEVEN ("7"),
    EIGHT ("8"),
    NINE ("9"),
    TEN ("10"),
    JACK ("J"),
    QUEEN ("Q"),
    KING ("K"),
    ACE ("A");

    private String symbol;

    Rank (String symbol ) {
        this.symbol = symbol;
    }

    /**
     * Returns a string representation of this rank which is the full name
     * where the first letter is capitalised and the rest are lowercase
     */
    @Override public String toString () {
        //only capitalize the first letter
        String s = super.toString();
        return s.substring (0, 1).toUpperCase() + s.substring(1).toLowerCase();
    }

    public String getSymbol () {
        return this.symbol;
    }
}

I think you should add a numeric field indicating the order of a card relative to the other cards. You do not need to use it for anything other than sorting.

Also, instead of taking the identifier of the enum value and doing uppercase and lowercase tricks with it in order to turn it into something presentable to the user, you should just store the name, too, as a separate string in the enum.

So, it would look like this: ACE( "A", "Ace", 0 ) (where 0 indicates lowest order for sorting.)

Furthermore you can also add one more number, the 'weight' of a card, to use in calculations which determine whether a card beats another. In general, enums in java are very powerful, so powerful that you pretty much end up not having to use the switch statement with enums anymore, because you can include a big part of an enum's functionality within the enum itself.