Schema.org microdata code check I updated my code based on the feedback from my previous question. There is one for the home page and one for the product pages:

Home page code:

NAME OF TRAVEL AGENCY
<link itemprop="url" href="HOME PAGE URL">
SHORT DESCRIPTION OF THE TRAVEL AGENCY

Product page code:

    <link itemprop="author" href="http://schema.org/TravelAgency" />
    <link itemprop="url" href="HOME PAGE URL">

    <link itemprop="specialty" href="http://schema.org/PlasticSurgery" />
    <meta itemprop="aspect" content="treatment"/>

    TITLE OF PAGE
    SHORT DESCRIPTION OF PAGE CONTENT

    The cost of
    NAME OF TREATMENT
    ALTERNATE NAME OF TREATMENT is €600.00

Home page (TravelAgency)

Looks correct.

Note that you could (if you want) use this item on every page of your website, not only on the home page.

Typically, you could always use this item for marking up the header of each page. By using its url property, you always link back to the same URL (i.e., your home page’s URL), so consumers can understand that it’s always the same TravelAgency entity.

Product page (MedicalWebPage)

Your use of the properties specialty, aspect, name, description, and image is correct.

author property

The value of the author property needs to be the URL of the author, not the URL of the Schema.org type.

Schema.org expects an Organization/Person as value, so you could make this explicit by using:

  <link itemprop="url" href="HOME PAGE URL" />

If you don’t want to use so "much" markup, you could also simply link directly:

<link itemprop="author" href="HOME PAGE URL" />

However, most sites have a link to the home page anyway (i.e., in the logo or the navigation), so you could reuse this link instead of duplicating it with link.

url property

The value of the url property would have to be the URL of this MedicalWebPage, not the URL of your homepage.

It doesn’t hurt to have it, but it isn’t really needed in the first place (as it’s just the URL of the current document anyway).

You could, if you want, combine this with a possible rel-canonical in your head.

offers property

Using the offers property on MedicalWebPage would mean that you are offering this web page (e.g., for sale), as its definition says (bold emphasis mine):

  An offer to provide this item […]

This is probably not what you want to say, or is it?

Offer

itemOffered property

The itemOffered property expects a Product as value. This is not required (using text, like you do, is allowed), but if you want to go the extra mile, make it a Product item:

  NAME OF TREATMENT

span element can’t have a content attribute

In HTML5/Microdata, only the meta element can have a content attribute, so it’s not allowed on span.

Instead of €, you might want to use:

<meta itemprop="priceCurrency" content="EUR">€

And instead of 600.00:

600.00


AppleScript to close running processes I've been trying to emulate MS Windows behavior on my OS X and close processes that do not have a window.

What I'd really like to do is "quit" the process on clicking the red "x" button.  Instead, I've managed to code this workaround, which constantly runs in the background.

(see my question here)

It kind of defeat the purpose of not having background apps running, I suppose.

Since I own my Mac for about 2 weeks, I'm not sure if my script is "good" in terms of memory consumption, etc. (right now: CPU = 0, Memory = 12.3 MB, Threads = 3).

How would you guys do it?  I'm open to suggestions.

#!/usr/bin/osascript

-- INICIO DAS FUNCOES EXTRAS
set app_path to path to current application
set app_name to get name of me
set myPath to path to me
tell application "Finder" to set myFolder to (container of myPath) as string
set commonScript to load script alias ((myFolder) & "FuncoesExtras.scpt")
-- FIM DAS FUNCOES EXTRAS

set WhiteList to {app_name, "App Store", "iTunes", "Finder", "Mail"}

repeat

    tell application "System Events"
        repeat with this_app in (get processes whose background only is false and windows is {})
            set NomeDoApp to the name of this_app
            if NomeDoApp is not in WhiteList then
                try
                    tell NomeDoApp to quit
                    log_event("App " & NomeDoApp & " encerrado com sucesso", "FecharProgramas") of commonScript
                on error
                    do shell script "killall " & quoted form of NomeDoApp
                    log_event("Forcando interrupcao do App " & NomeDoApp, "FecharProgramas") of commonScript
                end try
            end if
        end repeat
    end tell

    tell application "System Events" to set myPID to (unix id of processes whose name is app_name)
    do shell script ("/usr/bin/renice 18 " & myPID)

    delay 10

end repeat

Maybe a late response but I'm new to codereview but not to AppleScript. The problem with AppleScript and scripts that stays open is that they leak memory. Why? I just blame it on AppleEvents. It's a nice feature, it really is, but it does eats memory somehow. So the longer the script will run the more memory it will eat. Eventually, maybe after 6 months, your fans will start blowing for now reason and the mac becomes significant slower. But maybe it does that after a day, a week or a year. It's hard to tell in AppleScript where and how much a script leaks.

To solve this there are several ways to do this. You can do it with cron (is since Tiger built inside launchd) with a shell script or create a new process of itself and quit. Either way they all remove the repeat loop out of the AppleScript and let that part be handled by other, much better, software. Note: using an idle handler inside AppleScript will eat memory too which should be the nicest solution instead of using a repeat and a delay.

My advise, is that the script won't be eating to much memory so I would spawn a new process of itself just over some time. You're waiting 10 seconds so 360 loops would be enough. You spawn a new process around every hour.

change the repeat to repeat 360 times and add the following code to the end of your AppleScript file.

  do shell script "osascript " & quoted form of posix path of
  (path to me) & " &>/dev/null &"

Note: User interaction (display or choose commands) is not allowed when a script is launched by osascript in Mountain lion or earlier. For any user interaction change the target to SystemUIServer or Finder for instance.


Swapping two elements of a singly-linked list Similarly to Learning Rust With Entirely Too Many Linked Lists, I'm trying to learn Rust by working with linked lists. Currently I'm trying to implement a function bubble(...) that takes 2 elements of a singly-linked list and swaps them.

Any feedback is greatly appreciated! (Rust playground link here.)

#![allow(dead_code)]

use ::std::mem::replace;
use ::std::mem::swap;

#[derive(Debug)]
pub struct List<T> {
    list: Node<T>,
}

type Node<T> = Option<Box<Link<T>>>;

#[derive(Debug)]
struct Link<T> {
  head: T,
  tail: Node<T>,
}

impl<T> List<T> {
  pub fn push(&mut self, elem: T) {
    self.list = Some(Box::new(
      Link{ head: elem, tail: replace(&mut self.list, None) }));
  }

  pub fn pop(&mut self) -> Option<T> {
    match replace(&mut self.list, None) {
      Some(next_box) => {
        let next = *next_box;
        self.list = next.tail;
        Some(next.head)
      }
      _ => None
    }
  }

  // First attempt: Use push+pop. Not perfect, as we move the values
  // in and out, and if they're larger, we waste resources.
  pub fn bubble(&mut self) -> bool {
    if let Some(first) = self.pop() {
      if let Some(second) = self.pop() {
        self.push(first);
        self.push(second);
        return true;
      } else {
        self.push(first);
      }
    }
    false
  }

  // Learning from the above attempt, I created another push+pop
  // functions that don't move values, just Boxes instead.

  // Any tail of 'singleton' is silently discarded.
  fn push_singleton(&mut self, mut singleton: Box<Link<T>>) {
    swap(&mut self.list, &mut singleton.tail);
    self.list = Some(singleton);
  }

  fn pop_singleton(&mut self) -> Node<T> {
    match replace(&mut self.list, None) {
      Some(mut next_box) => {
        swap(&mut self.list, &mut next_box.tail);
        Some(next_box)
      }
      _ => None
    }
  }

  // Otherwise the implementation is very similar to 'bubble' above.
  pub fn bubble2(&mut self) -> bool {
    if let Some(first_box) = self.pop_singleton() {
      if let Some(second_box) = self.pop_singleton() {
        self.push_singleton(first_box);
        self.push_singleton(second_box);
        return true;
      } else {
        self.push_singleton(first_box);
      }
    }
    false
  }

  // Third attempt: Directly unpack the first two nodes and combine them
  // back together.
  pub fn bubble3(&mut self) -> bool {
    if let Some(mut first_box) = replace(&mut self.list, None) {
      if let Some(mut second_box) = replace(&mut first_box.tail, None) {
        first_box.tail = replace(&mut second_box.tail, None);
        second_box.tail = Some(first_box);
        *self = List{ list: Some(second_box) };
        return true;
      } else {
        *self = List{ list: Some(first_box) };
      }
    }
    false
  }
}

fn main() {}

Run Rustfmt. It well automatically tell you such things as:

Idiomatic Rust uses 4-space indents.
Match arms without curly braces have trailing commas
You don't need to prefix paths in use with ::; that's the default.

You don't need #![allow(dead_code)] and it's better to not disable warnings that broadly.
It's usual to import only the module, not the free function, and then namespace the function. For example, mem::replace.
mem::replace(&mut /* ... */, None) is equivalent to Option::take
You can also replace some usages of mem::swap with Option::take and direct assignment. This means you don't need to use functions from mem at all.

#[derive(Debug)]
pub struct List<T> {
    list: Node<T>,
}

type Node<T> = Option<Box<Link<T>>>;

#[derive(Debug)]
struct Link<T> {
    head: T,
    tail: Node<T>,
}

impl<T> List<T> {
    pub fn push(&mut self, elem: T) {
        self.list = Some(Box::new(Link {
            head: elem,
            tail: self.list.take(),
        }));
    }

    pub fn pop(&mut self) -> Option<T> {
        match self.list.take() {
            Some(next_box) => {
                let next = *next_box;
                self.list = next.tail;
                Some(next.head)
            }
            _ => None,
        }
    }

    // First attempt: Use push+pop. Not perfect, as we move the values
    // in and out, and if they're larger, we waste resources.
    pub fn bubble(&mut self) -> bool {
        if let Some(first) = self.pop() {
            if let Some(second) = self.pop() {
                self.push(first);
                self.push(second);
                return true;
            } else {
                self.push(first);
            }
        }
        false
    }

    // Learning from the above attempt, I created another push+pop
    // functions that don't move values, just Boxes instead.

    // Any tail of 'singleton' is silently discarded.
    fn push_singleton(&mut self, mut singleton: Box<Link<T>>) {
        singleton.tail = self.list.take();
        self.list = Some(singleton);
    }

    fn pop_singleton(&mut self) -> Node<T> {
        match self.list.take() {
            Some(mut next_box) => {
                self.list = next_box.tail.take();
                Some(next_box)
            }
            _ => None,
        }
    }

    // Otherwise the implementation is very similar to 'bubble' above.
    pub fn bubble2(&mut self) -> bool {
        if let Some(first_box) = self.pop_singleton() {
            if let Some(second_box) = self.pop_singleton() {
                self.push_singleton(first_box);
                self.push_singleton(second_box);
                return true;
            } else {
                self.push_singleton(first_box);
            }
        }
        false
    }

    // Third attempt: Directly unpack the first two nodes and combine them
    // back together.
    pub fn bubble3(&mut self) -> bool {
        if let Some(mut first_box) = self.list.take() {
            if let Some(mut second_box) = first_box.tail.take() {
                first_box.tail = second_box.tail.take();
                second_box.tail = Some(first_box);
                *self = List {
                    list: Some(second_box),
                };
                return true;
            } else {
                *self = List {
                    list: Some(first_box),
                };
            }
        }
        false
    }
}

fn main() {}

For what it's worth, I prefer bubble2 as the implementation is better factored and has helper functions.

I'm not a big fan of the word "singleton" here, as that feels like a misuse of the term. I don't have a great alternative name other than {push,pop}_boxed though.


Linked list implementation correctness I've started learning C++ and I wanted to ask you to take a look at my linked list implementation and give comments what's wrong with. It compiles and runs fine, but I am curious about const correctness, proper use of pointers/references, etc., so feel free to go super tough on me!

I've read about these concepts online but this doesn't get me far without an experienced C++ programmer's opinion.

Node:

#include <iostream>

#pragma once
template <typename T>
class Node
{
const char * const _key;
Node *_next;
T _value;

public:
Node(const char * const key, const T &value) : _key(key), _value(value), _next(nullptr) {}

const char * const getKey()
{
    return _key;
}

Node *getNext()
{
    return _next;
}

void setNext(Node *next)
{
    _next = next;
}

T &getValue()
{
    return _value;
}
};

Linked List:

#include <iostream>
#include "Node.h"

#pragma once

template <typename T>

class LinkedList
{
int _count;
Node<T> *_first;

public:
LinkedList() : _count(0), _first(nullptr) {}

Node<T> *add(const char * const key, const T &value)
{
    Node<T> *first = new Node<T>(key, value);
    first->setNext(_first);
    _first = first;
    _count++;
    return _first;
}

void remove(const char * const key)
{
    Node<T> *next = _first;
    Node<T> *previous = nullptr;

    while (next)
    {
        if (next->getKey() == key)
        {
            if (next == _first)
                _first = next->getNext();
            else
                previous->setNext(next->getNext());
            _count--;
            return;
        }
        previous = next;
        next = next->getNext();
    }
}

int getCount()
{
    return _count;
}

Node<T> * const getFirst()
{
    return _first;
}
};

I am not too certain how to properly define value inside Node class, because I want to support both primitive types and objects.

This actually worked for me, although this is probably not correct?

if (next->getKey() == key)

There are quite a few issues with your code as it stands. I know you've said to go "super tough" on you, but the amount of knowledge required to write a relatively complete linked list implementation in C++ is more than just about any other language.

As a minor point up-front, I'm going to change anything that has a prefixed _ (like _first or _key) to using a postfix underscore (so first_ and key_). The reason for this is that certain symbols are reserved for compiler usage; the rules are arcane, but the simplest thing to do is:

Avoid anything with a leading underscore (_<varname>)
Avoid anything with 2 trailing underscores (<varname>__)

Edit: As pointed out by @JerryCoffin, you should avoid __ just about anywhere (prefix, postfix, and so on).

The first (and largest) problem is that this never frees any of the heap allocated memory (that is, memory allocated with new). Every time you do:

Node<T> *first = new Node<T>(key, value);

This will allocate memory from the heap. Unlike garbage collected languages (most languages except C and C++), it is up to you, the programmer, to free this memory when you are done with it. Here, this means manually writing the destructor:

~LinkedList()
{
    Node<T>* next = _first;
    while(next != nullptr) {
        first_ = first_->next;
        delete next;
        next = first_;
    }
}

This will make sure that memory is reclaimed.

To go along with this, there are a few methods that are implicitly generated for you when you write a class in C++. These are the copy assignment operator, and the copy constructor. They have the forms:

LinkedList& operator=(const LinkedList& rhs); // Copy assignment
LinkedList(const LinkedList& rhs);  // Copy constructor

When are these called? Say you define a LinkedList<int>:

LinkedList<int> a;
// Do some operations on a, such as adding nodes
LinkedList<int> b;
// Some operations on b
b = a;  // Calls the copy assignment operator, operator=
LinkedList<int> c(a);   // Calls the copy constructor

The methods that C++ implements for you here will do something that you do not want them to do: the b.first_ pointer will be equal to a.first_. What this means is that, if the destructor for a is called, then b will be pointing to memory locations that have been reclaimed using delete (which is very, very bad).

To fix this, they need to be implemented to do the correct thing. So, what is the correct thing to do? Let's look at them each in turn:

The copy constructor just needs to copy each element of the passed in LinkedList in turn:

LinkedList(const LinkedList& rhs)
{
    Node<T>* tmp = rhs.first_;
    while(tmp != nullptr) {
        add(tmp->key_, tmp->value_);
    }
}

The copy assignment operator needs to:

Free the current memory held by this
Copy each value in the list rhs

So, the implementation looks something like:

LinkedList& operator=(const LinkedList& rhs)
{
    // Stop self assignment, like: a = a;
    if(&rhs != this) {
        // Step 1: Free current memory
        Node<T>* tmp = first_;
        while(tmp->next != nullptr) {
            first_ = first_->next;
            delete tmp;
            tmp = first_;
        }
        count_ = 0;

        // Step 2: Create a copy of rhs
        temp = rhs.first_;
        while(tmp != nullptr) {
            add(tmp->key_, tmp->value_);
        }
    }
    return *this;
}

This is actually a very well known thing in C++: it is called the rule of three (or in more modern C++, the rule of five - but ignore this for the moment). What it says is:

If you need to manually write any one of the destructor/copy constructor/copy assignment operator, then you need to manually define all three of them.

For a full treatment of this, you might want to read this post.

Using a const char * const as a key in a linked list is unusual, and doesn't really fit with the basic idea of the data structure. If you want to look something up by key, you're generally looking for a data structure called a map (or hashmap, or dictionary; there are multiple names for it). I'd remove the complication there, and just get rid of the key member entirely.

void add(const T &value)
{
    Node<T> *first = new Node<T>(key, value);
    first->next_ = first_;
    first_ = first;
    count_++;
}

I've also removed the return of the Node<T> *. You generally don't want to let the users of your class have direct access to a Node<T> *.

Your remove function also leaks memory, you again need to have a delete in there.

 Node<T>* tmp = next;
 if (next == _first) {
     _first = next->getNext();
     delete tmp;   // Need a delete here
 }
 else {
     previous->setNext(next->getNext());
     delete tmp;  // Or here
 }
 _count--;

Edit: How you've written a search is much more dictionary-like. For the standard library list, you'd often use something like std::find_if, which takes a predicate (which is just a fancy word for a function that returns a bool). This allows you to use an actual function to select what you're looking for. As an example, if you have a Student struct:

struct Student
{
    std::string name;
    int age;
    int year;
};

Then you had a list of Students:

std::list<Student> students;

You might want to search for a student who has the name "John": this would look like:

std::find_if(students.begin(), students.end(), [](const Student& s)
    { return s.name == "John"; });

In fact, this will actually return an iterator into the container.

This also gives you more flexibility, as you can use the same function to find a student who is, say, 17 years old:

std::find_if(students.begin(), students.end(), [](const Student& s)
    { return s.age == 17; });

Edit 2: As was pointed out, the copy assignment operator is not written in the best way. Firstly, understanding why this is the case is important. If you look at the steps we go through, we firstly free all the memory currently held, and then repopulate the current list. The problem with this is that if anything throws an exception during either of these, we'll be left with object state that is inconsistent. The way to fix this is to use what is commonly termed "copy and swap". This also has the nice property that it allows us some code reuse, cutting down on duplication.

The first port of call is to write a (member) swap function:

template <typename T>
void LinkedList<T>::swap(LinkedList<T>& other)
{
    using std::swap;
    swap(first_, other.first_);
    swap(count_, other.count_);
}

Then a free function that forwards to this:

template <typename T>
void swap(LinkedList<T>& a, LinkedList<T>& b)
{
    a.swap(b);
}

Now we can write a copy assignment operator as follows:

LinkedList& operator=(LinkedList rhs)
{
    swap(*this, rhs);
    return *this;
}

Note that the rhs parameter is now taken by value. This will perform a copy construction, hence once we enter the call, rhs will contain a full copy. Then all that needs to happen is a few pointer swaps. Note also that the original memory that was held is destroyed: when rhs goes out of scope at the end of the function, its destructor is called. Since it now points to (what was) the original memory for this, that memory is properly reclaimed.

This has a number of benefits: Firstly, it is succinct and reuses code, secondly, it is exception safe: if any operation throws, our object is left in a consistent state - the update is effectively atomic; it either works completely or fails completely.

Using swap, you can also easily write a move assignment operator.


Countdown timer in every table cell I am making countdown app with multiple records, so I am using UITableView to show that records and countdown dates. What I do is just take one timer and reload cells on every second. Is this a best approach to reload visible cells on every second or is there another better approach there?

updateCellContentsTimer = Timer.scheduledTimer(timeInterval: 1,
            target: self,
          selector: #selector(ViewController.updateCells),
          userInfo: nil, repeats: true)

// MARK: Custom Functions

func updateCells() {
    let indexPathsArray = tableView.indexPathsForVisibleRows
    for indexPath in indexPathsArray! {
        let cell = tableView.cellForRow(at: indexPath) as! CountdownTableViewCell

        cell.timeLeftLabel.text = progress + "hours:minutes:seconds"
    }
}

to avoid DRY i would put this line (i think it is in your code only a sample logic) inside the cell. in this case you have the logic for calculating the remaining time inside the cell and not outside somewhere. and you can use always the same function to update the progress.

cell.timeLeftLabel.text = progress + "hours:minutes:seconds"

should be become

cell.updateProgress()


Take all underlined words, put in Excel column I have a 100+ page Word document, that I would like to extract all of the underlined words from.  I am very familiar with VBA and Excel, but relatively new to Word. I was wondering if this is the best way to do what I'm trying.

I will run this from my Word document:

Sub extractUnderlinedWords()
Dim thisDoc As Word.Document
Application.ScreenUpdating = False
Dim appExcel As Object, oxlWbk As Object
Dim FN      As String
Dim aRange  As Range
Dim intRowCount As Integer
intRowCount = 1

Set thisDoc = ActiveDocument
Set aRange = thisDoc.Range

Set appExcel = CreateObject("Excel.application")
FN = "C:\Users\[blah]\UnderlinedWords.xlsx"

If FileExists(FN) Then
    Set oxlWbk = appExcel.workbooks.Open(fileName:=FN).Sheets("Sheet1")
End If

With aRange.Find
    Do
        .Font.Underline = True
        .Execute
        If .Found Then
            '   aRange.Expand Unit:=wdSentence
            '      aRange.Select
            If Len(aRange) > 1 Then
                If Not aRange.InRange(thisDoc.TablesOfContents(1).Range) Then
                    aRange.MoveEndWhile cset:=Chr(13), Count:=wdBackward
                    '    aRange.Copy
                    oxlWbk.Cells(intRowCount, 1).Value = aRange.Text
                    aRange.Collapse wdCollapseEnd
                    Debug.Print "Page: " & aRange.Information(wdActiveEndAdjustedPageNumber)
                    '    aRange.Select
                    If oxlWbk Is Nothing Then
                        intRowCount = 1
                    End If
                    'oxlWbk.Cells(intRowCount, 1).Value = aRange.Text
                    ' oxlWbk.Paste
                    intRowCount = intRowCount + 1
                End If
            End If
        End If
    Loop While .Found
End With
If Not oxlWbk Is Nothing Then
    appExcel.workbooks(1).Close True
    appExcel.Quit
    Set oxlWbk = Nothing
    Set appExcel = Nothing
End If
Set aRange = Nothing
Application.ScreenUpdating = True
MsgBox ("Done!")
End Sub

This does work, but after about page 14, it hangs up and "freezes" on me.

Should I instead, add all found words to an Array, then after it's done scanning the Word document, focus on Excel and outputting the parts of array row by row?  I think perhaps what's causing the freeze is the jumping back and forth from Word to Excel.  (Or, could it be that I'm using a Do/While loop?)

Again, the idea is to take all underlined words and put in an Excel sheet for reviewing.

In the Word document

Click on an underlined word
In the Home menu, in the Editing  section, click Select > Select Text with Similar Formatting
Copy
Open Excel and paste

You may need to clean it up in Excel, but you don't need VBA to do this.


Loading components on startup, with timing measurements I was just wondering, is there a way to load these better and log the times they each took? It seems like a headache having to write about 90+ stopwatches, which is what I will have to do as I have about 100 of these to load.

// PacketManager
var packetManagerStopwatch = Stopwatch.StartNew();
_packetManager = new PacketManager();
packetManagerStopwatch.Stop();
Logger.Trace("Loaded PacketManager [took " + packetManagerStopwatch.ElapsedMilliseconds + "ms]");

// Achievements
var achievementStopwatch = Stopwatch.StartNew();
_achievementManager = new AchievementManager();
achievementStopwatch.Stop();
Logger.Trace("Loaded Achievements [took " + achievementStopwatch.ElapsedMilliseconds + "ms]");

// Talents
var talentsStopwatch = Stopwatch.StartNew();
_talentTrackManager = new TalentTrackManager();
talentsStopwatch.Stop();
Logger.Trace("Loaded Talents [took " + talentsStopwatch.ElapsedMilliseconds + "ms]");

// Badges
var badgeStopwatch = Stopwatch.StartNew();
_badgeManager = new BadgeManager();
badgeStopwatch.Stop();
Logger.Trace("Loaded Badges [took " + badgeStopwatch.ElapsedMilliseconds + "ms]");

// ClientManager
var clientManagerStopwatch = Stopwatch.StartNew();
_clientManager = new GameClientManager();
clientManagerStopwatch.Stop();
Logger.Trace("Loaded GameClientManager [took " + clientManagerStopwatch.ElapsedMilliseconds + "ms]");

// ModerationManager
var moderationManagerStopwatch = Stopwatch.StartNew();
_moderationManager = new ModerationManager();
moderationManagerStopwatch.Stop();
Logger.Trace("Loaded ModerationManager [took " + moderationManagerStopwatch.ElapsedMilliseconds + "ms]");

// ItemDataManager
var itemManagerStopwatch = Stopwatch.StartNew();
_itemDataManager = new ItemDataManager();
itemManagerStopwatch.Stop();
Logger.Trace("Loaded ItemDataManager [took " + itemManagerStopwatch.ElapsedMilliseconds + "ms]");

You need a generic method like this:

static T createInstanceOf<T>() where T : new() {
  var stopwatch = Stopwatch.StartNew();
  T result = new T();
  stopwatch.Stop();
  Logger.Trace("Loaded "+ result.GetType().Name +" [took " + packetManagerStopwatch.ElapsedMilliseconds + "ms]");
  return result;
}

Now you can simply use it like this:

_packetManager = createInstanceOf<PacketManager>();
_achievementManager = createInstanceOf<AchievementManager>();
_talentTrackManager = createInstanceOf<TalentTrackManager>();
_badgeManager = createInstanceOf<BadgeManager>();
// and so on...


Counting the words in a textarea I've two working ways to do so, but which one should I use?

Common part: var textarea = document.getElementById("textarea");

First way:

function updateStatusBar() {
    var text = textarea.value;
    statusBar.value = "Words: " + (text ? text.match(/\b\S+\b/g).length : "0") +
      "  Characters: " + text.replace(/\s/g, "").length +
      " / " + text.replace(/\n/g, "").length;
}

and second way:

function updateStatusBar() {
    var text = textarea.value;
    statusBar.value = "Words: " + (text.split(/\b\S+\b/).length - 1) +
      "  Characters: " + text.replace(/\s/g, "").length +
      " / " + text.replace(/\n/g, "").length;
}

Please review the Words: counting code. Which one should I use?

The key difference between the two approaches is essentially these lines,
counting the words in text:

text ? text.match(/\b\S+\b/g).length : "0"
// ... versus ...
text.split(/\b\S+\b/).length - 1

First of all, the first expression will crash for a non-empty text without words, for example :!@#$. Because a non-empty text is "true", but the .match will return null, so you'll get a null pointer exception in .length.

Secondly, I suggest a simpler and more intuitive regular expression to match words:

/\w+/g

That is, match a non-empty sequence of word characters.

You could use this as text.match(/\w+/g) (notice the "g" flag) or as text.split(/\w+/). When using match, you need to check if the result is null or not, as you already did.

As for which way is better, using match or split,
I would argue for match:

It's more intuitive: it matches the character sequences you're interested in, and then count the occurrences.
It's probably more efficient: splitting implies creating an array of the results, but if you only need the size of the array (the count of elements), then it sounds like a waste.


Moving elements satisfying a predicate from one container to another I've implemented a C++ algorithm that moves elements satisfying a given unary predicate from one container into the other, deleting them from the input container.

This is my implementation:

/**
 * Moves all elements in the input container
 * that satisfy the given predicate
 * into the output container,
 * and erases them from the input container.
 * @tparam Container The container type.
 * @tparam UnaryPredicate The predicate type.
 * @param in The input container.
 * @param out The output container.
 * @param predicate The predicate to satisfy.
 */
template <class Container, class UnaryPredicate>
void move_and_erase_if(Container &in, Container &out, UnaryPredicate predicate) {
    // sort the input container so that all elements that
    // satisfy the predicate are moved to the end.
    std::sort(in.begin(), in.end(), [=](auto &a, auto &b){
        return !predicate(a) && predicate(b);
    });

    // find the first element in the sorted container
    // that satisfies the predicate.
    // all elements following that element
    // also satisfy the predicate.
    auto it = std::find_if(in.begin(), in.end(), [=](auto &val) {
        return predicate(val);
    });

    // move the elements satisfying the predicate
    // into the output container.
    std::move(it, in.end(), std::back_inserter(out));

    // erase the elements satisfying the predicate
    // from the input container.
    in.erase(it, in.end());
}

Is there a more efficient way to achieve this? Specifically, the call to std::find_if bugs me, as it applies the predicate to all elements a second time, until it finds the first one that satisfies it.

I've written an example application using the algorithm on ideone.

Sorting the source is not specified as a requirement, so it seems that std::partition would do the necessary job more efficiently. As a perk benefit, std::partition returns the partition point, eliminating the need to std::find_if.

If you still want to follow the sorting path, consider std::lower_bound instead of find_if.


Handling optional parameters in a servlet I have a form that submits to a servlet that has optional parameters. If they are empty they are just ignored. This is the template I am using:

if (!(param = (String)params.get("httpParam")).equals("")) {
   // Handle parameter
}

It doesn't quite feel right. It works, but I think it could be more readable. Any suggestions?

I suggest extract code for getting parameter to separate method (see extract method refactoring's tehnique):

param = getOptionalParameter("httpParam");
if (! param.isEmpty()) {
    // Handle parameter
}


Java method to load random paragraph from text file Looking for some critique for this code. but ultimately I would like to be directed to resources/books that address my code writing weaknesses, in terms of style structure an convention :

The task is to create code that would read a file that has several questions and answers, each question starts with the word "Question", and out put the question and a number of lines from the answer that is not greater than 40 lines.

my code method below is meant to:

read all lines into an List.
select a random start line in the list.
check id the line starting with the string "Question".
check if the remaining list elements are more than 40 (I've appended 40 empty lines to the end of the file, to make sure to include the last lines of the file).
create a string buffer to include maximum the 39 lines which are present after the start line, the one that contains "Question" , if the the word "Question" appears before the 40th line, then we we end the text captured at the preceding line (creating a text block).
return that (text block).

public static String readJavaInterview() throws IOException{

        List<String> list = Files.readAllLines(new File("src/lists/JavaCodingInterview.txt").toPath(), Charset.defaultCharset() );
        List<String> subList = new ArrayList<>();
        int start=0 , end= 0;

        Random rand = new Random();
        int n = rand.nextInt(list.size());
        StringBuffer textBlock= new StringBuffer();
        Pattern p = Pattern.compile("Question");
        Matcher m;

        textBlock.append("Some Java Interview Qs :\n");

        if (n + 40 < list.size()){

            subList = list.subList(n, n + 40);

            for (int i = 0 ; i < subList.size() - 1 ; i++){
                m = p.matcher(subList.get(i).toString());
                boolean b = m.find();

                if (b)
                    {

                    start= i + 1;
                    textBlock.append(subList.get(i) + "\n");
                    break;
                }else{
                    continue;
                }
            }

            for(int j = start ; j < start + 39; j++){
                m = p.matcher(subList.get(j).toString());
                boolean b = m.find();
                if (!b){

                    textBlock.append(subList.get(j) + "\n");

                }else{

                    break;
                }
            }

        }else{
            return "";
        }

        return textBlock.toString();
    }

Would this be how you would do it?

Some assumptions based on the limited explanation of the question you posted:

A new question starts with the word "Question" at the beginning of the line
The first line in the file starts with "Question". (not really required but otherwise we have to add a check).
A question can have any number of lines. It ends either at the end of the file, or right before a line that starts with the word "Question" (at which point the next question starts).
If a question contains more than 40 lines, we are only interested in the first 40. The rest of the question is ignored.
You want to randomly select a single question from the file as a result of the program

What I would do is split this up into 2 main parts.

1) read the file and split it up into separate "Questions"
2) select a random question

To me this looks a lot like I want to have List<Question> questions somewhere. Where Question is a new class to hold a single question.

Once we have that step 2 becomes almost trivial. (You've shown in your code you know how to do this).

Let's split step 1 up into specific parts as well.

1) read a file line by line
2) for each line:
2.a) Check if it is the start of a new Question.
2.b) add it to the current question if it's not 40 lines yet, otherwise ignore

Let's skip step 1 for a moment and look at how to process each line. (We'll come back to reading the file later).

I suggest creating a sort of "factory" class. This class will store that List<Question> that I wanted, and to make it easy for ourselves it will also store the Question currentQuestion. That way we can do step 2.b by just adding the line to the currentQuestion.

But first let's look at the Question class:

class Question {
    private List<String> lines = new ArrayList<>();

    public void addLine(String line){
        if(lines.size()>=40){
            return;
        }
        lines.add(line);
    }

    public List<String> getLines(){
        //we might want to make a new list here to prevent other classes from modifying it.
        return lines;
    }
}

Notice how I also made sure to only keep track of the first 40 lines. By doing this here, we can safely keep calling the add method while parsing all the lines without worying about it anymore.

Now for that factory class. This turns out to be rather easy as well:

public class InterviewQuestionFactory {
    private List<Question> questions = new ArrayList<>();
    private Question currentQuestion = null;

    public InterviewQuestionFactory(){
        //TODO read lines from file
        // for each ... {
        String line = "";
        process(line);
        //}
    }

    private void process(String line){
        if (line.startsWith("Question")) {
            currentQuestion = new Question();
        }
        currentQuestion.addLine(line);
    }
}

That was everything for step 2. All that's left is step 1 and we're done.

For step 1 here it's best to take a look at this post. Your current solution to read all lines at once might not work for large files.
Since java 8 you can easily do something like this instead:

try (Stream<String> lines = Files.lines(Paths.get(filename), Charset.defaultCharset())) {
    lines.forEachOrdered(line -> process(line));
}

And that's how I would have solved the problem. I left some parts up for you to actually complete but it should be almost trivial at this point.

To complete this review let's point out some things about your current piece of art:

doesn't work as intended

If your random start happens to be the line just after the start of a really long question you don't return anything.

unnecessary whitespace

this:

for (int i = 0 ; i < subList.size() - 1 ; i++){
    m = p.matcher(subList.get(i).toString());
    boolean b = m.find();

    if (b)
        {

        start= i + 1;
        textBlock.append(subList.get(i) + "\n");
        break;
    }else{

should look like this instead:

for (int i = 0; i < subList.size() - 1; i++) {
    m = p.matcher(subList.get(i).toString());
    boolean b = m.find();
    if (b) {
        start = i + 1;
        textBlock.append(subList.get(i) + "\n");
        break;
    } else {

variable names

In that last bit of code, what is the meaning of the variables m p b and start? This reads like:

You match the string from the sublist to something. If it matches you update start to the next index in the sublist and append the current String to the textblock.

Most of it can be solved by using sublist(i).startsWith("Question") instead. If we then also inline the b to get rid of it entirely and rename start to nextLineIndex it turns into this:

for (int i = 0; i < subList.size() - 1; i++) {
    if (sublist.get(i).startsWith("Question") {
        nextLineIndex = i + 1;
        textBlock.append(subList.get(i) + "\n");
        break;
    } else {

actually the start name was fine if you had set it to start = i instead. And start the following for loop at for(int j = start + 1; ... instead. At least then we would know you start looping at the second line of the question.

string.toString()

sublist.get(i) is already a String. No need to call toString() on it.


Infinite scroll pagination with a Wookmark layout /* Infinite Scroll Pagination with a Wookmark layout
 *
 * Requires: jQuery, Wookmark
 */

var myApp = myApp || {};

;(function(myApp,$){
  var WookmarkPagination = function(elem, options){
    this.elem = elem;
    this.$elem = $(elem);
    this.options = options;
  };

  WookmarkPagination.prototype = {
    defaults: {
      url: '',
      page: 2,
      per_page: 5,
      template: '',
      scroll: true,
      item: ".item",
      items: ".items",
      loader_image: ''
    },

    fetchReady: {
      status: true
    },

    /**
     * Initiate app. Bind the scroll event and onScroll function to the document.
     *
     * @return this
     */
    init: function(){
      var _this = this;
      _this.config = $.extend({}, _this.defaults, _this.options);
      var loader = '

';

      // Set loader
      $(loader).appendTo("body").hide();

      // Build wookmark layout
      wookmark_handler = this.$elem.find(_this.config.item);
      wookmark_handler.wookmark({
        container: _this.$elem,
        offset: 15,
        itemWidth: 160,
        autoResize: true
      });

      if(_this.config.scroll){
        $(document).bind('scroll', function(){
          _this.onScroll();
        });
      }
      return _this;
    },

    /**
     * Rebuild Wookmark layout
     *
     * @return void
     */
    refreshLayout: function(){
      var _this = this;
      var opt = _this.config;
      //console.log(wookmark_handler);

      // Rebuild wookmark positions
      if(wookmark_handler) wookmark_handler.wookmarkClear();
      wookmark_handler = _this.$elem.find(_this.config.item);

      // Initialize Wookmark to build layout container
      wookmark_handler.wookmark({
        container: _this.$elem,
        offset: 15,
        itemWidth: 160,
        autoResize: true
      });
    },

    /**
     * Fetches the JSON from the server. On success rebuilds Wookmark positioning.
     *
     * @return the JSON object
     */
    fetch: function(){
      var _this = this;
      var opt = _this.config;

      // If no other getJSON is being processed continue
      if(_this.fetchReady.status){
        // Block other requests
        _this.fetchReady.status = false;

        // Show loader
        $("#wookmark-pagination-loader").fadeIn();

        // Fetch Data
        return $.getJSON(opt.url,{
          page: opt.page,
          per_page: opt.per_page
        })
        .success(function(data){
          // Each data item is wrapped in a template and appended to the page.
          $.each(data, function(i, item){
            _this.$elem.find(_this.config.items).append(Mustache.to_html(opt.template, item.item));
          });

          // Set next page number for next request.
          opt.page += 1;

          _this.refreshLayout();

        })
        // TODO: setup error catching and show a user there was a problem with the request
        //.error(function(data, response){})
        .complete(function(data){
          // Allow other getJSON requests
          _this.fetchReady.status = true;

          // Hide loader
          $("#wookmark-pagination-loader").fadeOut();
        });
      }
    },

    /**
     * Checks if the scroll position has reached the bottom, then runs the fetch function
     *
     * @return void
     */
    onScroll: function() {
      // Check if we're within 100 pixels of the bottom edge of the broser window.
      var closeToBottom = ($(window).scrollTop() + $(window).height() > $(document).height() - 100);
      if(closeToBottom) {
        this.fetch();
      }
    }
  };

  WookmarkPagination.defaults = WookmarkPagination.prototype.defaults;

  $.fn.wookmark_pagination = function(options) {
    return this.each(function() {
      new WookmarkPagination(this, options).init();
    });
  };

  myApp.WookmarkPagination = WookmarkPagination;
})(myApp,jQuery);

The code looks pretty good, but here are a couple suggestions:

The semicolon at the top is unnecessary: ;(function(myApp,$){
You use _this in many places where this would work:

Consider Function.bind (see MDN, only works on modern browsers)
This very much depends on personal style

This is a little confusing. I don't write jQuery much, but it isn't clear what the first this is referring to (and the second this has a different meaning than the one above, which is also confusing):

$.fn.wookmark_pagination = function(options) {
  return this.each(function() {
    new WookmarkPagination(this, options).init();
  });
};


Tiny Lua library to get char pointer from string Background

I'm using Lua with luaglut to do some OpenGL stuff. The luaglut API is almost identical to the gl/glut C APIs. Sometimes, gl functions want a pointer to some data, for example:

glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, width, height, 0, GL_RGBA,
    GL_UNSIGNED_BYTE, oh_crap_a_pointer)

In these cases, the luaglut bindings take a light userdata containing the pointer. In the luaglut demos, a small example library called memarray does the work of setting up a C array and returning a pointer to it as a light userdata.

You might use "memarray" something like this:

require 'memarray'
function loadTexture(filename)
  local file = assert(io.open(filename, 'rb'))
  local data = file:read('*a')
  file:close()
  local array = memarray('uchar', #data)
  array:from_str(data)

  return array
end

-- ... later

local texture = loadTexture('whatever.rgba')
glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 32, 32, 0, GL_RGBA,
    GL_UNSIGNED_BYTE, texture:ptr())

That got me thinking, why do we need memarray at all? Lua strings should be binary safe; all we really need is to get a pointer to the string returned from
file:read('*a') as a light userdata. It would look something like this:

function loadTexture(filename)
  local file = assert(io.open(filename, 'rb'))
  local data = file:read('*a')
  file:close()

  return data
end

-- ... later

local stringpointer = require 'stringpointer'
local texture = loadTexture('whatever.rgba')
glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 32, 32, 0, GL_RGBA,
    GL_UNSIGNED_BYTE, stringpointer.get(texture))

Of course we pass the string around instead of the pointer, because the GC could free the memory the string was in if there are no more references to it, invalidating the pointer. Internally Lua strings are immutable and passed by reference anyway, so this is no big deal.

Code

#include <lua.h>
#include <lualib.h>
#include <lauxlib.h>

int get_pointer(lua_State *L)
{
    luaL_checktype(L, 1, LUA_TSTRING);
    lua_pushlightuserdata(L, (void *)lua_tostring(L, 1));
    return 1;
}

int luaopen_stringpointer(lua_State *L)
{
    const luaL_Reg api[] = {
        {"get", get_pointer},
        {NULL, NULL}
    };

#if LUA_VERSION_NUM == 501
    luaL_register(L, "stringpointer", api);
#else
    luaL_newlib(L, api);
#endif

    return 1;
}

It's really tiny, but my C is rusty so maybe I did something stupid. It seems to work fine so far. Any input is welcome.

I'm no Lua expert, but I can at least give you a pointer (no pun intended) about your C code.

Your get_pointer function looks like it works fine, but it could be made even more concise! Lua provides a luaL_checkstring function that combines luaL_checktype and luaL_tostring. Therefore, you could just do this:

int get_pointer(lua_State *L)
{
    lua_pushlightuserdata(L, (void *) luaL_checkstring(L, 1));
    return 1;
}


FileUpload to bitmap to base64String In my ASP.NET form I have a file upload control where I am letting users upload image files, converting it into base64String to store it in the database.

I am using following code to do so,

     if (FileUploadControl.HasFile)
            {
                var img = new Bitmap(FileUploadControl.FileContent);
                if (img.Height > 133 || img.Width > 133)
                {
                    img = ResizeImage(img, 132, 132);
                }

                u.Photo = ImageToBase64String(img);
            }

    private static string ImageToBase64String(Bitmap img)
    {
        var ms = new System.IO.MemoryStream();
        img.Save(ms, System.Drawing.Imaging.ImageFormat.Jpeg);
        byte[] byteImage = ms.ToArray();
        var base64String = Convert.ToBase64String(byteImage);
        return base64String;
    }

    private static Bitmap ResizeImage(System.Drawing.Image image, int width, int height)
    {
        var destRect = new Rectangle(0, 0, width, height);
        var destImage = new Bitmap(width, height);

        destImage.SetResolution(image.HorizontalResolution, image.VerticalResolution);

        using (var graphics = Graphics.FromImage(destImage))
        {
            graphics.CompositingMode = CompositingMode.SourceCopy;
            graphics.CompositingQuality = CompositingQuality.HighQuality;
            graphics.InterpolationMode = InterpolationMode.HighQualityBicubic;
            graphics.SmoothingMode = SmoothingMode.HighQuality;
            graphics.PixelOffsetMode = PixelOffsetMode.HighQuality;

            using (var wrapMode = new ImageAttributes())
            {
                wrapMode.SetWrapMode(WrapMode.TileFlipXY);
                graphics.DrawImage(image, destRect, 0, 0, image.Width, image.Height, GraphicsUnit.Pixel, wrapMode);
            }
        }

        return destImage;
    }

Now I am worried about followings,

Overall code review
Creating two Bitmap classes they are disposable but am not sure if I can dispose them at all
I seem to be doing unnecessary conversion because I don't have enough knowledge.

Goal

Convert uploaded images to 132 x 132 if they are over it and save them as base64String.

In ImageToBase64String() you do not dispose your stream. GC will do it but server-side is never never a good idea to postpone resource deallocation:

static string ImageToBase64String(Bitmap img)
{
    using (var ms = new System.IO.MemoryStream())
    {
        img.Save(ms, System.Drawing.Imaging.ImageFormat.Jpeg);
        return Convert.ToBase64String(ms.ToArray());
    }
}

Note that if you use this feature often you may reduce memory pressure reusing same buffer, you may implement an ObjectPool<MemoryStream> class.

You create a new bitmap just for resizing then yes, it has to be disposed too:

private static string ResizeAndConvertToBase64(Stream stream)
{
    using (var image = new Bitmap(FileUploadControl.FileContent))
    {
        if (IsTooBig(image))
        {
            using (var scaledImage = ScaleDownImage(image))
            {
                return ImageToBase64String(scaledImage);
            }
        }

        return ImageToBase64String(image);
    }
}

private static bool IsTooBig(Bitmap bitmap)
{
    return bitmap.Width > MaximumWidth || bitmap.Height > MaximumHeight;
}

Note I also moved magic numbers from code to constants MaximumWidth and MaximumHeight. Now you still have to deal with ScaleDownImage(). First of all you may want to keep image aspect ratio then I'd first move this calculation to another separate function (I dropped settings unused for image drawing):

private static Bitmap ScaleDownImage(Bitmap image)
{
    var newSize = ResolveScaledDownImageSize(image);
    var scaled = new Bitmap(newSize.Width, newSize.Height, PixelFormat.Format32bppArgb);

    using (var g = Graphics.FromImage(scaled))
    {
        g.CompositingQuality = CompositingQuality.HighQuality;
        g.InterpolationMode = InterpolationMode.HighQualityBicubic;
        g.SmoothingMode = SmoothingMode.AntiAlias;
        g.PixelOffsetMode = PixelOffsetMode.HighQuality;

        g.DrawImage(image, new Rectangle(Point.Empty, newSize));
    }

    return scaled;
}

There is also a built-in method to re-scale images, you should check if it satisfy your needing, in this case drop ScaleDownImage() and change ResizeAndConvertToBase64() as follow:

private static string ResizeAndConvertToBase64(Stream stream)
{
    using (var image = new Bitmap(FileUploadControl.FileContent))
    {
        if (IsTooBig(image))
        {
            using (var scaledImage = new Bitmap(image, ResolveScaledDownImageSize(image)))
            {
                return ImageToBase64String(scaledImage);
            }
        }

        return ImageToBase64String(image);
    }
}

In general don't forget to always call Dispose() for classes which implement IDisposable and whenever possible use using statement to simplify that code.

Now only ResolveScaledDownImageSize() function is empty. If you do not need to keep aspect ratio you may simply:

private static ResolveScaledDownImageSize(Bitmap image)
{
    return new Size(
        Math.Min(MaximumWidth, image.Width),
        Math.Min(MaximumHeight, image.Height));
}

To keep aspect ratio you may keep width or height (according to which one is bigger than required), you can use one of these to perform calculation:

\$height_{new} = \frac{height}{width} width_{new}\$

or

\$width_{new} = \frac{width}{height} height_{new}\$


similar code in several methods I have method which checks period and return schedules:

public IEnumerable<EventSchedule> GetSchedulesForPeriod(PeriodEvent period, string tab = "")
        {
            switch (period)
            {
                case PeriodEvent.Today:
                    return GetTodaySchedules(tab);

                case PeriodEvent.Tomorrow:
                    return GetTomorrowSchedules(tab);

                case PeriodEvent.Week:
                    return GetWeekSchedules(tab);

                case PeriodEvent.FewWeek:
                    return GetFewWeekSchedules(tab);

                case PeriodEvent.Month:
                    return GetMonthSchedules(tab);
                case PeriodEvent.All:
                    return GetAllFromTodaySchedules(tab);

                default:
                    return GetTodaySchedules(tab);
            }
        }

Consider the GetTodaySchedules and GetTomorrowSchedules:

    private IEnumerable<EventSchedule> GetTodaySchedules(string tab)
{
    var today = DateTime.Now.Date;

    var result = Database.EventSchedules.Where(s => s.RecurrenceStart.Value.Date <= today &&
         s.RecurrenceEnd.Value.Date >= today &&
         s.BaseEvent.IsApproved.Value && !s.IsRemoved.Value &&
         s.BaseEvent.EventsCategories.Any(
         c => c.EventCategory.Name == tab)).ToList();

    return result.Where(s => Evaluator.CheckDate(s, today)).ToList();
}
private IEnumerable<EventSchedule> GetTomorrowSchedules(string tab)
{
    var today = DateTime.Now.AddDays(1).Date;
    var result = Database.EventSchedules.Where(s => s.RecurrenceStart.Value.Date <= today &&
         s.RecurrenceEnd.Value.Date >= today &&
         s.BaseEvent.IsApproved.Value && !s.IsRemoved.Value &&
         s.BaseEvent.EventsCategories.Any(
         c => c.EventCategory.Name == tab)).ToList();

    return result.Where(s => Evaluator.CheckDate(s, today)).ToList();
}

The similar code have all methods above. How to rewrite it? May be, there is  some pattern for this?
And, the second, now I want that tab checks only if it filled and introduce new var called placeName (also check if filled):

private IEnumerable<EventSchedule> GetTodaySchedules(string tab, string placeName)
    {
        var today = DateTime.Now.Date;

        var result = Database.EventSchedules.Where(s => s.RecurrenceStart.Value.Date <= today &&
             s.RecurrenceEnd.Value.Date >= today &&
             s.BaseEvent.IsApproved.Value && !s.IsRemoved.Value &&
             s.BaseEvent.EventsCategories.Any(
             c => c.EventCategory.Name == tab) && s.BasePlace.Name).ToList();

        return result.Where(s => Evaluator.CheckDate(s, today)).ToList();
    }

I don't want to create separate methods for this. What is best way to do this?

Thanks.

You can compose query predicates by applying several where clauses in a row.
The predicates must be passed as Expression<Func<T, bool>> for composition.
See LINQ to Entities: Combining Predicates on Microsoft Docs.
Create a general schedules query like this
private IEnumerable<EventSchedule> GetSchedules(
    string tab,
    Expression<Func<EventSchedule, bool>> dateCondition)
{
    var result = Database.EventSchedules
        .Where(s => s.BaseEvent.IsApproved.Value &&
            !s.IsRemoved.Value &&
            s.BaseEvent.EventsCategories.Any(c => c.EventCategory.Name == tab))
        .Where(dateCondition)
        .ToList();
    return result.Where(s => Evaluator.CheckDate(s, today)).ToList();
}

And then pass it the appropriate individual date predicate like this
private IEnumerable<EventSchedule> GetTodaySchedules(string tab) {
    var today = DateTime.Now.Date;
    return GetSchedules(
        tab,
        s => s.RecurrenceStart.Value.Date <= today &&
             s.RecurrenceEnd.Value.Date >= today);
}

private IEnumerable<EventSchedule> GetTomorrowSchedules(string tab) {
    var tomorrow = DateTime.Now.AddDays(1).Date;
    return GetSchedules(
        tab,
        s => s.RecurrenceStart.Value.Date <= tomorrow &&
             s.RecurrenceEnd.Value.Date >= tomorrow);
}


Prettify phone numbers I am a java developer and I am trying to learn Go.
I tried to make a simple code that take numbers from a .txt file and make print them in a prettier format.
That is:

0613228745     -> 06 13 22 87 45
06.13.22.87.45 -> 06 13 22 87 45
06-13-22-87-45 -> 06 13 22 87 45
06 13 228 745  -> 06 13 22 87 45
0 613 228 745  -> 06 13 22 87 45

Can you review, give me some advices about my code ?
Here it is:

package main

import (
    "bufio"
    "fmt"
    "os"
    "unicode"
)

func check(e error) {
    if e != nil {
        panic(e)
    }
}

func main() {

    file, err := os.Open("./phonenumbers.txt")
    check(err)

    scanner := bufio.NewScanner(file)
    for scanner.Scan() {
        basicPhoneNumberArray, prettyPhoneNumber := make([]rune, 0), ""
        // get only digit numbers
        for _, r := range []rune(scanner.Text()) {
            if unicode.IsDigit(r) {
                basicPhoneNumberArray = append(basicPhoneNumberArray, r)
            }
        }
        // put spaces every two digits
        for i, r := range basicPhoneNumberArray {
            prettyPhoneNumber += string(r)
            if i%2 != 0 && i != len(basicPhoneNumberArray)-1 { // no space at the end
                prettyPhoneNumber += " "
            }
        }
        fmt.Println(prettyPhoneNumber)
    }
}

Also I am very fond of functional programming, I think there should be a way to use some filter and map here but after few searches I didn't find a way. If you could give me some advices about that too that would be great :D.

If you don't want to handle an error, then ignore it by using the _ symbol.
I.E. replace:

file, err := os.Open("./phonenumbers.txt")
check(err)

with:

file, _ := os.Open("./phonenumbers.txt")

Move the entire scanning logic into a separate function (you did write you are fond of functional programming ).
func PrettifyNumber(original string) string
You could use the strings.Fields to filter out any non numeric character.

func IsNotADigit(r rune) bool {
    return !unicode.IsDigit(r)
}

func PrettifyNumber(original string) string {
    return strings.Join(strings.FieldsFunc(original, IsNotADigit), " ")
}


Stack implementation using arrays Please comment on the same. How can it be improved?

Specific improvements I am looking for:

Memory leaks
C++ style
Making code run faster
RAII

#include<iostream>
#include<algorithm>

template <typename T>
class Stack {
private:
    T* array_;
    int length_;
    T* last_;
    void expandArray();

public:
    Stack(int length = 8) :
        array_(new T[length]),
        length_(length),
        last_(array_)
    {}

    Stack<T>& push(const T&);
    Stack<T>& pop();
};

template<typename T>
void Stack<T>::expandArray() {
    T* array_temp = new T[length_ << 1];
    std::copy(array_, array_ + length_, array_temp);
    std::swap(array_, array_temp);
    delete[] array_temp;
    last_ = array_ + length_ - 1;
    length_ <<= 1;
}

template<typename T>
Stack<T>& Stack<T>::push(const T& data) {
    if (last_ == (array_ + length_ - 1)) {
        expandArray();
    }
    last_[0] = data;
    last_++;
    std::cout << "[" << data << "] pushed." << std::endl;
    return *this;
}

template<typename T>
Stack<T>& Stack<T>::pop() {
    if(array_ != last_) {
        last_--;
        std::cout << "[" << last_[0] << "] popped." << std::endl;
    } else {
        std::cout << "Nothing to pop." << std::endl;
    }
    return *this;
}

int main() {
    Stack<std::string> s;
    s.push(std::string("a"))
     .push(std::string("b"))
     .push(std::string("c"))
     .push(std::string("d"));
    s.pop().pop().pop().pop().pop();
}

First of all, I would specify that your stack is using a "dynamically allocated array", as opposed to just an array, as I at first expected a C array when looking at the title.  That varies from person to person, though.

On to more important things:

I would not print anything in the stack functions.  If someone wants information on what they're pushing and popping, let them do it themselves.
You seem to be missing a peek function.
Either throw an exception or add an assert to pop for the array_ == last_ case.
length_ should actually be called capacity_.
expandArray may leak memory if the copy throws.  I would use an std::vector<T*> internally for this stuff.
You don't check that length > 0 in the constructor.
I would not allocate so anything by default.
You lack all three of a copy constructor, assignment operator, and destructor, so you'll definitely leak memory.  You should also have a move constructor and a move assignment operator, but those aren't as critical.
You provide no way to check whether the stack is empty.


indexOf Boyer-Moore String Search algorithm This is an implementation of Boyer-Moore string search algorithm index_of for a pattern in text. I was challenged to implement it by my friend.

This Python function returns the starting index of the first occurrence of a pattern in text and returns None if no index is found. For example, index_of('ababc', 'abc') returns 2 and index_of('abc', 'z') returns None.

I have the following implementation that handles most of the cases and I think the worst case running time is \$O(m/n)\$.

def index_of(text, pattern):
    assert isinstance(text, str), 'text is not a string: {}'.format(text)
    assert isinstance(pattern, str), 'pattern is not a string: {}'.format(text)
    # Check if empty; if so, return 0, earliest bit
    if len(pattern) == 0:
        return 0
    pattern = list(pattern)
    pattern_len = len(pattern)

    text = list(text)
    textext_len = len(text)

    shift = 0
    while(shift <= textext_len - pattern_len):
        index = pattern_len - 1
        # Keep going until it all meets
        while index >= 0 and pattern[index] == text[shift+index]:
            index -= 1
        # If we get past index, then return where the shift is
        if index < 0:
            return shift
        else:
            if text[shift+pattern_len-1] in pattern:
                shift += pattern_len - pattern.index(text[shift+pattern_len-1]) - 1
            else:
                shift += pattern_len
    return None

I include my code here. I ran my code against 3 separate tests, and it passed all of the pytests. So it's pretty robust, and I think my implementation works fine.

import unittest

class Test(unittest.TestCase):

    def test_index_of_with_matching_patterns(self):
        assert index_of('abc', '') == 0  # all strings contain empty string
        assert index_of('abc', 'a') == 0  # single letters are easy
        assert index_of('abc', 'b') == 1
        assert index_of('abc', 'c') == 2
        assert index_of('abc', 'ab') == 0  # multiple letters are harder
        assert index_of('abc', 'bc') == 1
        assert index_of('abc', 'abc') == 0  # all strings contain themselves
        assert index_of('aaa', 'a') == 0  # multiple occurrences
        assert index_of('aaa', 'aa') == 0  # overlapping pattern
        assert index_of('thisisabsolutelycrazy', 't') == 0
        assert index_of('abcdef', 'abcdef') == 0  # all strings contain
        assert index_of('abcdef', 'abcdef') == 0  # all strings contain

    def test_index_of_with_non_matching_patterns(self):
        # Negative test cases (counterexamples) with non-matching patterns
        assert index_of('abc', 'z') is None  # remember to test other letters
        assert index_of('abc', 'ac') is None  # important to test close cases
        assert index_of('abc', 'az') is None  # first letter, but not last
        assert index_of('abc', 'abz') is None  # first 2 letters, but not last

    def test_index_of_with_complex_patterns(self):
        # Difficult test cases (examples) with complex patterns
        assert index_of('ababc', 'abc') == 2  # overlapping prefix
        assert index_of('bananas', 'nas') == 4  # overlapping prefix
        assert index_of('abcabcabc', 'abc') == 0  # multiple occurrences
        assert index_of('abcabcab', 'abc') == 0  # multiple occurrences
        assert index_of('abcabcdef', 'abcd') == 3  # overlapping prefix
        assert index_of('abcabcdef', 'abcdef') == 3  # overlapping prefix
        assert index_of('abcabcdabcde', 'abcde') == 7  # overlapping prefix
        assert index_of('abcabcdabcde', 'abcd') == 3  # multiple occurrences, overlapping prefix
        assert index_of('abra cadabra', 'abra') == 0  # multiple occurrences
        assert index_of('abra cadabra', 'adab') == 6  # overlapping prefix

if __name__ == '__main__':
    unittest.main()

1. Bug

The code in the post does not work!

>>> index_of('abba', 'bba')
>>> # expected 1 but got None

Read on to see how you could have discovered this.

2. Review of the unit tests

Having a suite of unit tests is excellent, better than 99% of submissions to Code Review.
Using assert for the unit tests means that you don't get much information out of a test case failure. For example, if I artifically introduce a bug into the code, then I might get this test output:

======================================================================
FAIL: test_index_of_with_matching_patterns (cr179311.Test)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "cr179311.py", line 38, in test_index_of_with_matching_patterns
    assert index_of('abc', 'c') == 2
AssertionError
----------------------------------------------------------------------

At this point it would be helpful to know what the incorrect value was. The unittest.TestCase class has a bunch of assert methods that produce better output. For example, if I replace the line:

assert index_of('abc', 'c') == 2

with:

self.assertEqual(index_of('abc', 'c'), 2)

then the test failure message includes the incorrect value:

======================================================================
FAIL: test_index_of_with_matching_patterns (cr179311.Test)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "cr179311.py", line 38, in test_index_of_with_matching_patterns
    self.assertEqual(index_of('abc', 'c'), 2)
AssertionError: -1 != 2
----------------------------------------------------------------------

Each unit test knows its expected result. But you had to type these in by hand and could have made a mistake. It would be more reliable if the expected result were computed. In this case we could use the built-in str.find method, for example:

self.assertEqual(index_of('abc', 'c'), 'abc'.find('c'))

This approach is known as using a test oracle.
The unit tests are very repetitive. Every test has the form:

assert index_of(haystack, needle) == expected_index

or if you follow my suggestions above,

self.assertEqual(index_of(haystack, needle), str.find(haystack, needle))

The repetition could be avoided by making the tests data-driven, like this:

class TestIndexOf(unittest.TestCase):
    CASES = [
        # Positive cases
        ('abc', ''),             # all strings contain empty string
        ('abc', 'a'),            # single letters are easy
        ('abc', 'b'),
        ('abc', 'c'),
        ('abc', 'ab'),           # multiple letters are harder
        ('abc', 'bc'),
        ('abc', 'abc'),          # all strings contain themselves
        ('aaa', 'a'),            # multiple occurrences
        ('aaa', 'aa'),           # overlapping pattern
        ('thisisabsolutelycrazy', 't'),
        ('abcdef', 'abcdef'),    # all strings contain themselves
        ('ababc', 'abc'),        # overlapping prefix
        ('bananas', 'nas'),      # overlapping prefix
        ('abcabcabc', 'abc'),    # multiple occurrences
        ('abcabcab', 'abc'),     # multiple occurrences
        ('abcabcdef', 'abcd'),   # overlapping prefix
        ('abcabcdef', 'abcdef'), # overlapping prefix
        ('abcabcdabcde', 'abcde'), # overlapping prefix
        ('abcabcdabcde', 'abcd'), # multiple occurrences, overlapping prefix
        ('abra cadabra', 'abra'), # multiple occurrences
        ('abra cadabra', 'adab'), # overlapping prefix

        # Negative cases
        ('abc', 'z'),           # remember to test other letters
        ('abc', 'ac'),          # important to test close cases
        ('abc', 'az'),          # first letter, but not last
        ('abc', 'abz'),         # first 2 letters, but not last
    ]

    def check(self, haystack, needle):
        expected = haystack.find(needle)
        if expected == -1:
            expected = None
        self.assertEqual(index_of(haystack, needle), expected,
                         "index_of({!r}, {!r})".format(haystack, needle))

    def test_cases(self):
        for haystack, needle in self.CASES:
            self.check(haystack, needle)

This makes it easy to add more test cases.
Since we have a test oracle, it would make a lot of sense to generate random test cases. This allows us to explore a much larger space of possibilities than we could do just by writing out test cases by hand.

It's easy to generate random (positive) test cases:

def test_random(self):
    for n in range(1, 100):
        haystack = ''.join(random.choice('abc') for _ in range(n))
        start, stop = sorted(random.sample(range(n + 1), 2))
        needle = haystack[start:stop]
        self.check(haystack, needle)

And if we run these randomly generated test cases, we soon get a test failure as described in §1 above:

======================================================================
FAIL: test_random (cr179311.TestIndexOf)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "cr179311.py", line 91, in test_random
    self.check(haystack, needle)
  File "cr179311.py", line 81, in check
    "index_of({!r}, {!r})".format(haystack, needle))
AssertionError: None != 3 : index_of('cacaaaaaa', 'aaaaaa')
----------------------------------------------------------------------


Finding missing numbers in an array Is it possible to make this faster? Any suggestions are more than welcome
I have used JavaScript to write this code
PROBLEM

You will get an array of numbers.

Every preceding number is smaller than the one following it.

Some numbers will be missing, for instance:
  [-3,-2,1,5] // missing numbers are: -1,0,2,3,4

Your task is to return an array of those missing numbers:
[-1,0,2,3,4]

SOLUTION
const findTheMissing = (target) => {

    // final result list
    let result = [];

    // array will go from min to max value present in the array
    const min = target[0];
    const max = target[target.length - 1];

    // will maintain the track of index of target array
    // will start from 2nd element of array because we need a no. to subsctract from
    let pivotIndex = 1;

    for (let index = min; index < max; index++) {

        // value to the index
        let pivotValue = target[pivotIndex];

        // dif of the value
        let diff = pivotValue - index;

        // diff means its time to move the pivot to next :P
        if (diff > 0) {
            // not going to current index at exists in the target array
            if (index === target[pivotIndex - 1])
                index++;

            // YO!! WE FOUND HE MISSING
            result.push(index);
        }
        else {
            pivotIndex++;
        }
    }
    return result; // got all missing numbers
}

RESULT
let source = [-5, 0, 2, 5, 7];
console.log(findTheMissing(source));
// [ -4, -3, -2, -1, 2, 3, 4, 6 ]

I think, there is an error:

If I enter the values:

[-3,-2,1,5]

I get the result:

[-2,0,2,3,4]

but it should be:

[-1,0,2,3,4]

The error occurs when there are two adjacent numbers in the array (here -3 and -2).

Naming:

Index is not an index, but the actual value in the valid sequence (from min to max). I would call it value or curValue or something like that.

I don't like the name target either, because the target is actually the sequence of numbers from min to max. input would be better IMO.

Analysis

Basically you want to compare two sequences of numbers index by index. If they differ then save the target value, if not then increment to the next value of the input array. The index of the target sequence (here from min to max) is always incremented. Instead of the target sequence you can just increment a value starting from input[0] and ending on input.[input.length - 1].

All in all it could be done something like this:

function findMissing(input) {

  var result = [];

  for (var inputIndex = 0, targetValue = input[0]; targetValue <= input[input.length - 1]; targetValue++) {
    if (input[inputIndex] != targetValue) {
      result.push(targetValue);
    }
    else {
      inputIndex++;
    }
  }

  return result;
}


Compiler warning on unused exception...need to improve structure? Consider the below:

public int DoSomethingComplicated(ComplicatedObject input)
{
    try {
        return input.Analyse();
    }
    catch(CustomExceptionOne ex1)
    {
         throw;
    }
    catch(CustomExceptionTwo ex2)
    {
         Log(ex2.Message);
         return -1;
    }
}

Obviously I have abstracted quite a bit for the example, but the idea is that the try block performs a task which can cause one of two types of runtime exception.

In the event that the exception type is CustomExceptionOne, I want to simply throw the exception. For CustomExceptionTwo on the other hand, I want to log the issue but return -1.

I do not have access to the input.Analyse() method in order to change it.

This all looks fine to me, and works fine, but I get a compiler warning because ex1 is technically never used. I don't really want to throw the entire ex1 exception, so I am wondering how I can get this warning to go away.

I can think of a couple of hacky ways like Console.WriteLine(ex1.Message) or something but that isn't a route I particularly want to go down.

You can refactor your code the following way:

public int DoSomethingComplicated(ComplicatedObject input)
{
    try
    {
        return input.Analyse();
    }
    catch(CustomExceptionTwo ex2)
    {
         Log(ex2.Message);
         return -1;
    }
}

throw; re-throws the same exception, so there is no point in such catch block.

Alternatively, you can use catch(CustomExceptionOne) syntax.


My Memory Game "Engine" Mostly for the experience (but also so I could waste hours playing it), I made a simple memory card game in JavaScript. I built it so it has a backbone framework that could be easily extended by stripping off my actual implementation. It's the backbone I'd like reviewed here, and mostly:

My OO - anything wrong; anything that could be improved on?
Obvious code efficiency: is there anything that could make it more efficient?
Structure: Is the structure of the game a good idea?

var Card = (function() {
    var self = Object.create({}, {
        val: {
            value: -1
        },
        index: {
            value: -1
        },
        addTo: {
            value: function(game) {
                var random = -1;
                counter = 0; //break counter to stop infinite loop. :)
                while ((game.cards[random] !== undefined && counter <= 100) | random === -1) {
                    random = Math.round(game.cards.length * Math.random());
                    counter++;
                }
                this.index = random;
                game.cards[random] = this;
            }
        },
        isMatch: {
            value: function(game) {
                if (this.val == game.selected.val) {
                    game.matches++;
                    return true;
                }
                return false;
            }
        }
    });
    return self;
})();
var Game = (function() {
    var self = Object.create({}, {
        cards: {
            value: new Array(30)
        },
        matches: {
            value: 0
        },
        init: {
            value: function(func) {
                for (i = 0; i < this.cards.length / 2; i++) {
                    var card = Object.create(Card, {
                        val: {
                            value: i
                        }
                    });
                    var card2 = Object.create(Card, {
                        val: {
                            value: i
                        }
                    });
                    card.addTo(this);
                    card2.addTo(this);
                }
                if (typeof func === 'function') {
                    func();
                }
            }
        },
        selected: {
            value: null
        }
    });
    return self;
})();

Sorry about the length; that's partly why I want it reviewed.

To see how to implement it and the full code, see here

EDIT: FYI, I realize the game isn't currently extensible.  For example, if I wanted there to be multiple types of matches, I can't do that quite yet. I'm pretty sure I know how to add that in, but I wanted to make sure my design structure was solid first.

First I see that both classes depend on each other and are tightly coupled together.  Why does a Card object need to know its own index in an Array?

Secondly, the Card object updates the game object, but the game object contains an array of the cards, which means the Game object owns a bunch of Card objects.

If you were to remove the dual dependency, then you could rewrite your isMatch method to be

    isMatch: {
        value: function(inCard) {
            return (this.val == inCard.val);
        }
    }


Critique my first C++ "Hello World" program This is my first attempt to write a simple C++ program:

#include <iostream>
#include <cstring>
#include <vector>
using namespace std;

class Book {
    public:
    char title[30];

    Book(char *tit)  {
        strcpy(title,tit);
    };

    char *gettitle() {
        return title;
    }
};

int main() {

    Book miolibro((char *)"aaa");
    cout << miolibro.gettitle() << endl;

   std::vector<Book> vettorelibri;
   for (int i=0; i<10; i++) {
        Book l( (char *)"aaa" );   // i tried something like "aaa"+i but it does not work

        vettorelibri.push_back(l);
    }

    for(int i=0; i<10; i++) {
        cout << vettorelibri[i].gettitle() << endl;
    }
};

It has been commented that when working in C++ you should use std::string.  Assuming you want to stay with C style strings (or at least understand how you ought to work with them, in case you ever have to), I have the following suggestions:

strcpy(title,tit);

tit can be larger than the buffer title allows.[1]  In that case, your program will crash or worse, continue running while corrupting some other part of memory.  To avoid that you should always use functions which let you track the size of the destination buffer and detect overflow.  For example:

strncpy(title, sizeof(title)-1, tit);
title[sizeof(title) - 1] = 0;

This has some undesirable traits in that it will silently truncate the string if the input is larger than the destination.  If you are working on a system that has the non-standard strlcpy, that has some cleaner error conditions:

if (strlcpy(title, sizeof(title), tit) >= sizeof(title)))
{
    // TODO: decide how to handle the error (in your case maybe throw
    // an exception)
}

Moving along the spectrum (from code that crashes given large input, to code that truncates the input, to code that errors out when the buffer in insufficient...), you might also want to dynamically allocate memory for this.

char *title;

Book(const char *tit)
{
   title = strdup(tit);
   if (!title) { throw std::bad_alloc(); }
}

This creates complications if a Book object is ever copied, so you need to handle that:

Book(const Book &b)
{
   title = strdup(b.title);
   if (!title) { throw std::bad_alloc(); }
}

And it needs to be freed when your object is destroyed:

~Book()
{
   free(title);
}

If this seems too complicated it's because for higher-level C++ code it probably is.  std::string will take care of all of this for you.  Note though that the memory layout of your object will be substantially different with these dynamic-memory approaches.[2]

Next up...

Book miolibro((char *)"aaa");

As a general rule, having to do casts like this is often an indicator that you're doing something wrong.  :-)  When writing code and reviewing the work of others' I tend to try to avoid as many unnecessary casts as possible.  A lot of times these mask bugs, for example casting a function to an incorrect function pointer type can produce crashes, or casting away const may crash when someone tries to modify a read-only buffer.  For reasons like this it's a good habit to watch for inappropriate casts when reading code, and be sure not to introduce them yourself; code like this should offend your sensibilities.  :-)  [3]

In your case, the issue is that string literals have the type const char*, so the compiler complains that you are passing this to something that doesn't have const.  So what you need to do here is:

Book(const char *tit)
{
    // ...
}

Footnotes and tangential comments:
[1] If there are any English speaking people who might read your code, I suggest you come up with a better name than tit for that variable.
[2] In the earlier example, characters of title exists alongside any other members of the class.  When using dynamic memory, there's now an extra pointer dereference going on to get to the characters.  In most cases this won't make a difference but you can think of theoretical cases where the former is desired over the latter.
[3] Side note: In my experience one way to tell that someone doesn't know what they're doing in C/C++ is the use of casts.  A sloppy programmer will write code that the compiler rejects, and then to get it "working" quickly they'll introduce casts to silence compiler warnings instead of fixing the actual problem.  If you are working on a team where people do this, I find it's sometimes a good indication to be skeptical of that person's work for other reasons.


Is this an efficient merge sort implementation with regards to average time complexity in Big-O notation? Is my below merge sort implementation in O(n log n)? I am trying to implement a solution to find k-th largest element in a given integer list with duplicates with O(N*log(N)) average time complexity in Big-O notation, where N is the number of elements in the list.

 public class FindLargest {
        public static void nthLargeNumber(int[] arr, String nthElement) {
            mergeSort_srt(arr, 0, arr.length - 1);
            // remove duplicate elements logic
            int b = 0;
            for (int i = 1; i < arr.length; i++) {
                if (arr[b] != arr[i]) {
                    b++;
                    arr[b] = arr[i];
                }
            }

            int bbb = Integer.parseInt(nthElement) - 1;
            // printing second highest number among given list
            System.out.println("Second highest number is::" + arr[b - bbb]);
        }

        public static void mergeSort_srt(int array[], int lo, int n) {
            int low = lo;
            int high = n;
            if (low >= high) {
                return;
            }

            int middle = (low + high) / 2;
            mergeSort_srt(array, low, middle);
            mergeSort_srt(array, middle + 1, high);
            int end_low = middle;
            int start_high = middle + 1;
            while ((lo <= end_low) && (start_high <= high)) {
                if (array[low] < array[start_high]) {
                    low++;
                } else {
                    int Temp = array[start_high];
                    for (int k = start_high - 1; k >= low; k--) {
                        array[k + 1] = array[k];
                    }
                    array[low] = Temp;
                    low++;
                    end_low++;
                    start_high++;
                }
            }
        }

        public static void main(String... str) {
            String nthElement = "2";
            int[] intArray = { 1, 9, 5, 7, 2, 5 };

            FindLargest.nthLargeNumber(intArray, nthElement);
        }
    }

Is my below merge sort implementation in O(n log n)?

No, while your merge sort appears to be a good implementation of the in-place merge sort, that sort has a complexity of O(n log2n ). While the complexity may look reasonably scalable, the actual surt is quite slow compared to others. Specifically, it is a much slower sort than the native TimSort used by the Collections API in Java7 and above.

@tintinmj is right to challenge the use of a String input parameter for nthElement. It should be a simple int input parameter.

You should note that you could get to the nthElement faster by moving backwards from the end of the sorted data. If you move forwards you have to scan the entire array. If you move backwards, you only have to go as far as the matching value.

OK, so the input parameters are of the wrong type, the sort is more expensive than anticipated, and the final scan could be faster ... but, the real question I have is why not just scan the data once and be done with it....

private static final int nthLagest(int[] data, int nth) {
    int[] result = new int[nth];
    int rescnt = 0;
    for (int d : data) {
        // read the documentation for binarySearch, and why I do ` - xxx - 1`
        int ip = - Arrays.binarySearch(result, 0, rescnt, d) - 1;
        if (ip >= 0) {

            // this value is not recorded... which may be because it is good,
            // or because it is not better than the nth.
            if (rescnt < result.length) {
                // we don't have nth large numbers yet.
                System.arraycopy(result, ip, result, ip + 1, rescnt - ip);
                rescnt++;
                result[ip] = d;
            } else if (ip > 0) {
                // this value is larger than one of the nth values we already have.
                ip = ip - 1;
                System.arraycopy(result, 1, result, 0, ip);
                result[ip] = d;
            }
        }
    }
    if (rescnt < result.length) {
        throw new IllegalStateException("There is no " + nth + " largest value in " + Arrays.toString(data));
    }
    return  result[0];
}

The above function takes the data, and scans through it just once. As it goes, it keeps track of the highest 'n' numbers, where 'n' is the nth largest number you want to get.

The overall complexity for this function is something like O(n log m) where n is the number of elements in the input data, and m is the input value nth (if you want to get the third-largest value, then m = 3).


Identifying a development and production server by http_host I am using below code to recognize which set up I need to use in Codeigniter:

$dev_urls = array('dev.com');
$prod_urls = array('prod.com');

if(in_array($_SERVER['HTTP_HOST'], $dev_urls)){
    define('ENVIRONMENT', 'development');
}
else if(in_array($_SERVER['HTTP_HOST'], $prod_urls)){
    define('ENVIRONMENT', 'production');
}
else{
    define('ENVIRONMENT', '');
}

if (defined('ENVIRONMENT'))
{
switch (ENVIRONMENT)
{
    case 'development':
        error_reporting(E_ALL);
        require('/application/config/development/dev_config.php');
    break;

    case 'testing':
    case 'production':
        error_reporting(0);
        require('/application/config/production/prod_config.php');
    break;

    default:
        exit('The application environment is not set correctly.');
}
}

I went through this answer, but, I am still skeptical about my code.

Switching behaviour based on the Host header is not something I would recommend.  The environment should be a property of the server, rather than of the request.  Depending on your hosting situation (whether you have an IP-based virtual host or name-based virtual host, and the details of how your webserver is configured), you might even be able to hit code on the production server using a request with a forged (or missing) Host header.

I recommend that you identify the production environment by some other means — probably by some configuration file that exists on the production server.


Create object-properties from an array I'm writing a simple table-oriented storage for objects. The schema of a table is stored in the columns-array and the data in the rows-array.

// example: table "employees"
var columns = ["id", "name"]
var rows = [[1, "Alice"], [2, "Bob"]];

I wrote a function to convert a row-array into an object:

function convertRowToObject(row) {
    var result = {};
    for (var i = 0; i < columns.length; i++) result[columns[i]] = row[i];
    return result;
}

Now I can get an employee-object where I can access the fields by name, not by index.

// [1, "Alice"] -> { id: 1, name: "Alice" }

Is there any way to get the convertRowToObject function any smaller? I think there should be a way to get rid of the loop and make this with a single line of code.

You could make it smaller by turning your function into a constructor:

function RowObject(row) {
    for (var i = 0; i < columns.length; i++)
      this[columns[i]] = row[i];
}

You would have to call this function with new then. I would avoid putting the assignment on the same line as the for, it is too Golfic to maintain.

The only way to avoid a loop is to fake it:

function convertRowToObject2(row) {
  //o -> object, v -> value, i -> index
  return row.reduce( function(o, v, i){ o[columns[i]] = v; return o; } , {} );
}


Game server auth: cookie key generate&check UPD: Haskell version, please check it too.

gameKey(long id) is authentication function of an MMO game server. When someone wants to play with friends, he sends request to server, server calls gameKey() and returns it as JSON to player via API. When a friend establishes connection, he sends the game id and his key, server calls gameKey(id), where id is user data, and then checks user's key.

Algorithm is:

use current time and internal counter to generate id if it's not provided, or use provided id
make fast crypto hash using Blake2 function of id and secret(Yes, I know about HMAC and using as key, but in this case it doesn't matter)
split 64-bit hash to four 16-bit values, one hash for one user
make JSON object from id and keys and return it

I want to make stable code: exploit-free, no memory leaks and high speed (for high load and DDOS prevention). I'm experimenting now, so the algorithm was first written in JavaScript, because I'm skilled in JS coding. Then I rewrote JS code in C++ to increase execution speed. You can see C++ and JS code below. The C++ code is 3x faster than JS: 34s vs 10s, per 1M keys

Please review the C++ code; I think it can be significantly improved (I'm bad in C++). Code compiles and works same as JS. Also please advise me, which programming language is better for this: JS, C++ , Haskell or Go?

Here's the C++ code; it requires libb2-dev and libjsoncpp-dev (run sudo apt install libb2-dev libjsoncpp-dev to install it on Ubuntu):

//"g++"  "main.cpp" -o "main" -lb2 -ljsoncpp
#include <sstream>
#include <string>
#include <iostream>
#include <ctime>
#include <blake2.h>//libb2-dev
#include <jsoncpp/json/json.h>//libjsoncpp-dev
#include <iomanip>

char TOPSECRET[] = "TOPSECRET";
long dateCounter = 0;

std::string gameKey(long id2=-1){

  long long id = std::time(0)*1000+dateCounter;
  if(id2!=-1){
    id = id2;
  }
  dateCounter++;

  std::stringstream inStream;
  inStream << id << "|keys|" << TOPSECRET;
  const std::string inString = inStream.str();
  const char* in = inString.c_str();

  size_t outlen=64;
  uint8_t* out = new uint8_t[outlen];
  char key[] = "";
  blake2b(out,in,&key,outlen,inString.length(),0);

  std::stringstream a,b,c,d;
  for(int i=0;i<16;i++){
    a << std::setfill('0') << std::setw(2) << std::hex << static_cast<unsigned>(out[i]);
  }
  for(int i=16;i<16*2;i++){
    b << std::setfill('0') << std::setw(2) << std::hex << static_cast<unsigned>(out[i]);
  }
  for(int i=16*2;i<16*3;i++){
    c << std::setfill('0') << std::setw(2) << std::hex << static_cast<unsigned>(out[i]);
  }
  for(int i=16*3;i<16*4;i++){
    d << std::setfill('0') << std::setw(2) << std::hex << static_cast<unsigned>(out[i]);
  }

  Json::Value obj(Json::objectValue);
  obj["id"] = id;

  Json::Value arr(Json::arrayValue);
  arr.append(a.str());
  arr.append(b.str());
  arr.append(c.str());
  arr.append(d.str());
  obj["keys"] = arr;

  Json::Writer* writer = new Json::FastWriter();
  std::string jsonOut = writer->write(obj);

  delete writer;
  //delete in;//comment=memory leak, uncomment=segfault when gameKey() is called more than once
  delete out;

  return jsonOut;
}

int main(){
  std::cout << gameKey();
  std::cout << gameKey();
}

Here's the node.js version; it needs blakejs (run npm i blakejs to install it):

const TOPSECRET = "TOPSECRET", blake = require("blakejs").blake2bHex;
let dateCounter = 0;

function gameKey(checkId){
  let id = checkId || (Date.now()/1000>>0)*1000+(dateCounter++%1000),
    keys = blake(`${id}|keys|${TOPSECRET}`).match(/.{32}/g);
  return {id, keys};
}
console.log(gameKey());

We have better random generators in C++ than std::time(0); if you're using the latter for its speed, it would really be worth a comment in the code to explain that you're willing to live with such a weak random-number source in exchange for the speed.

Note that embedding the secret in the code will make it visible to anyone who can obtain the source or the binary and read it.  I'd recommend that you read it at runtime from a separate file - it's easier to correctly permission that one file rather than your source and all the build products.

For the case where the id is specified, there's no need to compute a time-based one just to throw it away immediately - I'd turn that conditional around:

std::string gameKey(long long id = -1)
{
  if (id == -1) {
    id = std::time(0) * 1000 + dateCounter++;
  }

Alternatively, write an overloaded function pair:

std::string gameKey()
{
   return gameKey(std::time(0) * 1000 + dateCounter++);
}

std::string gameKey(long long id)

A couple of misspellings: std::size_t and std::uint8_t (both missing namespace prefix in the code - it might work on your platform, but it's not portable).

I think that key can be a const char* or even a string literal:

blake2b(out, in, "", outlen, inString.length(), 0);

General: please give your code a bit more room to breathe - bunching up the source with no whitespace won't make it any more efficient.

Given that outlen is constant, we could declare it constexpr - then we wouldn't need to allocate out, but could declare it in automatic storage.  There's certainly no need to allocate writer and have to delete it - just make it an automatic variable:

Json::Writer writer;
return writer.write(obj);
// look, no delete needed!

If you do need a variable outlen, at least use a smart pointer, so it's cleaned up for you.

Instead of writing to 4 buffers a, b, c and d, it might be simpler to write to a single buffer, and copy substrings from there (it would reduce those repeated loops with similar-but-different ranges).