Implementation of LinkedList C++ using friend I am implementing my own linkedlist in C++ as part of a class assignment. My main concern isn't with the style of the code nor the pros/cons of including or not including certain features as I am still developing member functions.

I would mainly like to know if my approach of declaring LinkedList as a friend of Node is reasonable. My decision for doing this comes from the fact that there is absolutely no need for anything to access my Node class besides LinkedList (e.g. I don't want to be able to create a Node object in main). When I do this, however, I find myself no longer needing public and private access modifiers for Node. Furthermore, I don't particularly need getter/setter methods as I can just access Node's member variables directly from LinkedList (you can see below that I don't call Node::getVal() or Node::getNext() once).

Coming from the world of Java, it seems really strange to ever access an object's internal data directly without getters/setters. What do you guys think? Does this friend relation make sense?

template <typename T> class Node
{
    friend class LinkedList<T>;
    // private:
        Node<T>* next;  // CAN ACCESS MEMBERS DIRECTLY BECAUSE OF FRIEND DECLARATION
        T value;
    // public:
        Node<T>(T valuep)
        {
            value = valuep;
            next = NULL;
        }
        void insert(T valuep)
        {
            if(next == NULL)
            {
                next = new Node<T>(valuep);
                return;
            }
            else
            {
                next->insert(valuep);
            }
        }

        T getVal() // DON'T NEED BECAUSE OF FRIEND DECLARATION
        {
            return value;
        }
        Node<T>* getNext() // DON'T NEED BECAUSE OF FRIEND DECLARATION
        {
            return next;
        }
};

template <typename T> class LinkedList
{
    private:
        Node<T>* head;
    public:
        LinkedList<T>()
        {
            head = NULL;
        }
        ~LinkedList<T>()
        {
            Node<T>* currentNode = head;
            while(currentNode!=NULL)
            {
                Node<T>* next = currentNode->next;
                delete currentNode;
                currentNode = next;
            }
        }
        void insert(T valuep)
        {
            if (head == NULL)
            {
                head = new Node<T>(valuep);
                return;
            }
            else
            {
                head->insert(valuep);
            }
        }

        void print()
        {
            Node<T>* temp = head;
            while(temp!=NULL)
            {
               std::cout << temp->value << std::endl; // ACCESS Node's MEMBERS DIRECTLY
               temp = temp->next; // ACCESS Node's MEMBERS DIRECTLY
            }
        }

        void update(int indexp, T valuep)
        {
            if(indexp==0)
            {
                head->value = valuep; // ACCESS Node's MEMBERS DIRECTLY
            }
        }
};

You could do this:

template<typename T> class LinkedList {

    struct LinkedListNode {
        T m_element;
        LinkedListNode* m_next;

        LinkedListNode(T element)
        :
        m_element{element},
        m_next{nullptr}
        {}
    };

public:

    LinkedList()
    :
    m_head{nullptr},
    m_tail{nullptr}
    {}

    void add(T element) {
        LinkedListNode* node = new LinkedListNode(element);
    }

private:
    LinkedListNode* m_head;
    LinkedListNode* m_tail;
};

int main() {
    LinkedList<int> l;
}

That way, your LinkedListNode implementation is not available outside of your LinkedList.


You should probably fire the Database Admin.

But seriously, a problem in your query arises if you have duplicate, but different (bank_id + branch_id)'s. For example, (22 + 202) and (2 + 2202) both equal the same thing.

Change your query to include the space, instead of replacing it on both sides.

Here's a SQL Fiddle demonstrating the issue:
http://sqlfiddle.com/#!18/3a6b9/2

Example query to match exactly, don't remove the spaces (Note won't work on SQL fiddle as I used different lengths for the colunmns):

SELECT BANK_ID, BRANCH_ID, ACCOUNT_NUM, CURRENCY, AMOUNT
FROM [ACCOUNTS]
  JOIN  [AMOUNTS]
  ON (
    CAST(
         CAST  ([BANK_ID] as  nchar(2)) + ' '
       + CAST  ([BRANCH_ID] as  nchar(3)) + ' '
       + CAST  ([ACCOUNT_NUM] as  nchar(4))
        AS varchar(255)
        )
         LIKE
        CAST ([AMOUNTS].[ACCOUNT_REC] AS varchar(255))
     )

Update, since you mentioned more about your schema:

https://dbfiddle.uk/?rdbms=sqlserver_2017&fiddle=f8c037b9cc9afc2e2de850011031020d

SELECT  * FROM ACCOUNTS JOIN AMOUNTS ON (
         CONCAT(CONVERT(VARCHAR(255), [BANK_ID]), ' ')
       + CONCAT(CONVERT(VARCHAR(255), [BRANCH_ID]), ' ')
       + CONVERT(VARCHAR(255), [ACCOUNT_NUM])
       ) LIKE ([AMOUNTS].[ACCOUNT_REC] )


Iterate only once using LINQ Funny thing, the following test works:

    [TestMethod]
    public void Iterate_Once()
    {
        var x = new[] { 1, 2, 3, 4, 5, 6 }.ToIterable();
        CollectionAssert.AreEqual(new[] { 1, 2 }, x.Take(2).ToArray());
        CollectionAssert.AreEqual(new[] { 3, 4, 5 }, x.Take(3).ToArray());
    }

Where:

static class Iterable
{
    public static IEnumerable<T> ToIterable<T>(this IEnumerable<T> source)
    {
        var it = source.ToList().GetEnumerator();
        return once();

        IEnumerable<T> once()
        {
            while (it.MoveNext())
                yield return it.Current;
        }
    }
}

That should be safe to do not dispose an iterator of in-memory collection.

Please do not put me in jail for that :)

So what you are trying to do is to have an implementation of IEnumerable<T> which has a singleton IEnumerator<T>? Why not implement that explicitly, instead of using undocumented (?) behaviour of List<T>'s iterator?

Simply create a class that consumes a IEnumerable<T>, takes its iterator and wraps it in an implementation of IEnumerator<T> that has a dysfunctional Reset method. IEnumerator<T>.GetEnumerator() can then return the instance to that same iterator instead of creating a new one for each call. It could look something like this:

  public class OnlyOnceIterator<T> : IEnumerable<T>, IEnumerator<T> {
    private readonly IEnumerator<T> enumerator;
    internal OnlyOnceIterator(IEnumerable<T> sequence) {
      enumerator = sequence.GetEnumerator();
    }

    public T Current => enumerator.Current;

    object IEnumerator.Current => enumerator.Current;

    public void Dispose() {
      enumerator.Dispose();
    }

    public IEnumerator<T> GetEnumerator() => this;

    public bool MoveNext() {
      return enumerator.MoveNext();
    }

    public void Reset() {
      return;
    }

    IEnumerator IEnumerable.GetEnumerator() => this;
  }

This could then be used in an extension method:

  public static OnlyOnceIterator<T> ToOnlyOnceIterator<T>(this IEnumerable<T> sequence) {
    if (sequence == null) {
      throw new System.ArgumentNullException(nameof(sequence));
    }

    return new OnlyOnceIterator<T>(sequence);
  }

Additionally, I would prefer a more specific return type than IEnumerable<T>, since the returned behaviour is significantly different to what any user might expect from IEnumerable<T>. Consider creating a new interface that inherits from IEnumerable<T> with a more descriptive name. Through inheritance, Linq will still be able to be used, and you can create methods that only accept this specific interface, (compare this to IOrderedEnumerable<T>).


Custom maximumBy and minimumBy in Haskell Going through the magnificent Haskell Programming from first principles, I'm currently reimplementing functions from standard libraries such as maximumBy and minimumBy.

Here are my current versions of them:

myMaximumBy :: (a -> a -> Ordering) -> [a] -> a
-- maximumBy in Data.List also throws an error on empty lists
myMaximumBy _ [] = error "empty list"
myMaximumBy f (x:xs) =
  foldr (\curMax x -> if f x curMax == GT then x else curMax) x xs

myMinimumBy :: (a -> a -> Ordering) -> [a] -> a
myMinimumBy f = myMaximumBy $ flip f

I know they're not total functions, but I'll get to rewriting them with something like Maybe later on.

I'm happy for any kind of feedback on how to make the code shorter, more readable, or more idiomatic.

Since maximumBy folds a list of elements into a single element, it seems like a perfect candidate to use foldr or foldr1. However, we would need a -> a -> a instead of a -> a -> Ordering.

Let's have a look at maximum first, which can be defined via max :: Ord a => a -> a -> a:

maximum :: (Foldable t, Ord a) => t a -> a
maximum = foldr1 max

That looks simple enough to adapt to our myMaximumBy:

myMaximumBy :: (a -> a -> Ordering) -> [a] -> a
myMaximumBy f = foldr1 (maxBy f)

We now only need a maxBy that transforms an a -> a -> Ordering into a a -> a -> a:

maxBy :: (a -> a -> Ordering) -> a -> a -> a
maxBy comp x y = if x `comp` y == LT then y else x

Compared to your original solution, we now

replaced explicit recursion by foldr1
introduced a helper function that can get reused

Our new version is now easier to maintain and use.


Solving the sliding pyramid problem in Python3 A simple set of functions which solves the sliding pyramid problem. In summary, the task is to find the path from top to bottom of a triangular grid of numbers that has the smallest sum. The program uses the file input.txt as input.

from time import time

def build_tree():
    with open("input.txt", "r") as file:
        lines = file.readlines()
    list_tree = []
    for line in lines[1:]:
        list_tree.append([int(n) for n in line.strip().split(" ")])

    return list_tree

def min_sum(tree):
    doug = list(reversed(tree))
    above = doug[0]
    for row in doug[1:]:
        below = row
        for index, n in enumerate(below):
            below[index] += min(above[index],above[index+1])
        above = below

    return above[0]

if __name__ == "__main__":
    a = time()
    tree = build_tree()
    print(min_sum(tree), time() - a)

These samples are simple, but I appreciate any feedback.

My main critiques are:

doug is a baffling variable name.
You shouldn't have to store the entire file in memory, nor should you have to invert the triangle.  Doing so hurts scalability.  You should be able to process each line as you read it.

Instead of your build_tree() implementation, I recommend using a generator function to parse the input, such that you only need to store one line at a time.

min_sum() could be made more elegant without relying so much on indexes.

def triangle_rows(filename):
    with open(filename) as f:
        int(next(f))    # Skip first line, which specifies the number of rows
        for line in f:
            yield [int(entry) for entry in line.split()]

def min_sum(triangle):
    row = next(triangle)
    for row_below in triangle:
        row = [min(l, r) for l, r in zip(
            [a + b for a, b in zip(row + [row[-1]], row_below)],
            [a + b for a, b in zip([row[0]] + row, row_below)]
        )]
    return min(*row)

if __name__ == '__main__':
    print(min_sum(triangle_rows('input.txt')))


Move two element of an array to the front - Javascript I want to move two element of my array to the beginning.
This solution works but I would like to know a better approach since using sort() twice is probably not the best.
Array Input:
colors = [{name: yellow}, {name: blue}, {name: green}, {name: red}, {name: orange}]

JS Function:
sort(){
let green= colors.find(color=> color.name === "green")
let red= colors.find(color=> color.name === "red")

colors.sort(function(x,y){ return x == red? -1 : y == red? 1 : 0; });
colors.sort(function(x,y){ return x == green? -1 : y == green? 1 : 0; });
}

Output:
colors = [{name: green}, {name: red}, {name: yellow}, {name: blue}, {name: orange}]

PS: I want green to always the above red

Find the index of the green and red items, then remove them from the array with splice. Then you can make a properly ordered array by combining the removed items with the rest of the items in the original array:

const colors = [{name: 'yellow'}, {name: 'blue'}, {name: 'green'}, {name: 'red'}, {name: 'orange'}]
const removeColor = colorToRemove => (
  colors.splice(
    colors.findIndex(({ name }) => name === colorToRemove),
    1
  )[0]
);
const green = removeColor('green');
const red = removeColor('red');
const newColors = [green, red, ...colors];
console.log(newColors);

Or, without mutation (it's good to avoid mutation when not necessary, it can make code a bit more predictable in most cases):

const colors = [{name: 'yellow'}, {name: 'blue'}, {name: 'green'}, {name: 'red'}, {name: 'orange'}]
const getColor = colorToRemove => colors.find(({ name }) => name === colorToRemove);
const green = getColor('green');
const red = getColor('red');
const newColors = [green, red, ...colors.filter(({ name }) => name !== 'green' && name !== 'red')];
console.log(newColors);

Also note:

String literals require delimiters. {name: yellow} won't work - you want {name: 'yellow'}.
If you aren't going to reassign a variable name (most of the time you should be able to avoid reassignment), always use const. Only use let when you must reassign.
For readability, better to indent your code when entering a new block (such as a function).


Search iteratively in a ternary search tree I was trying to write an iterative search function for my Ternary Search Tree class based on a pseudo-code, and now it seems to work, but I think it can definitely be improved.

def search(self, key: str):
    return TST._search(self.root, key)

@staticmethod
def _search(node: TSTNode, key: str):
    """Note that node.key is a character"""
    if node is None or not key:
        return None

    for i in range(len(key) - 1):

        while node is not None and key[i] != node.key:
            if key[i] < node.key:
                node = node.left
            else:
                node = node.right

        if node is None:  # Unsuccessful search
            return None
        else:
            node = node.mid

    # In case the length of the key is 1 (but NOT only, see edit below)
    return node.value if node.key == key else None

Is my algorithm correct? My algorithm does not allow keys or characters to be empty strings.

Edit 1

I think there's a small bug in the code above in the last statement, which should be:

if not node or node.key != key[-1]:
    return None
else:
    return node.value

This is because there's one more way to arrive at that return statement (apart from having initially the key of length 1, as I wrote in the comment), which is to exit from the while loop because the condition key[i] != node.key is false.

In that case, we would be comparing a character, i.e. node.key, with a string key (if the node is not None: see note below!). In Python this can be easily hidden leading to bugs like this one.

Note also that node could still be None (I think) after we do node = node.mid.

Edit 2

After testing this method, I noticed that the last correction of the previous edit didn't solve all problems with the function. The last problem is that we still may need to go left or right after we exit the for loop. Here you can find the correct version:

  https://github.com/nbro/ands/blob/master/ands/ds/TST.py

Iterables

Generally speaking, if you use range(len(some_iterable))and then access an index of the item, you can just iterate over the item itself:

for i in range(len(word)):  # okay
    print(word[i])

for character in word:  # better
    print(character)

In your case it'd be key[:-1]

Style

Good on you for following PEP8. The code looks beautiful. There are more blank lines than I usually see, but they work.

The ternary operator (x if y else z)

Your use of the ternary operator is quite elegant here, but be aware that this makes your code harder to test.

I often see code that use the ternary operator as a sort of cheat to get out of unit test coverage (coverage.py doesn't notice if only one of the cases in a ternary statement is executed).

The algorithm

It looks fine to me. I'm not certain about the correctness. Can you link that pseudo-code you're working from?

You can make it a bit more obvious if you like. How do I call this method? What kind of object is key? I'm sure this makes complete sense in context with the rest of the class, but you can always add argument annotations and function annotations to make your code that much more readable:

def nice_number(x: float) -> str:
    ...

or

def f(n: 'must be prime'):
    ...

What can you do to improve this?

Write a unit test. It makes it so much easier to play with the implementation, to try out different things, without having to worry about breaking some edge case. Write a complete unit test, and be aware of what the algorithm can an can't do.

Run profiling. I don't think you will learn anything shocking by profiling the code, but it's always good to get a sense of what takes the most amount of time. My guess is that the slowest part of your code, considering how straightforward it is, will be the attribute access (node.key). (Rule of thumb: dots are expensive)

Write a docstring. Run help() on your module and try to imagine what it looks like to users.

Get rid of the wrapper method. Unless there's something I'm not seeing here, there's no reason to have a static method TST._search(), not visible to help() because of that underscore, and a wrapper method TST.search() that merely calls it.

If you do get rid of the static method, start off by setting node = self.root (going back to the earlier comment about dots being expensive, you don't want to have that in the hot part of your loop if avoiding it is so easy).

I almost never use staticmethod() in real-world code. If I do, I make sure to spend a minute thinking about whether it's really necessary.


Negative factorial in JavaScript I was doing a basic problem on coding a factorial \$n!\$:

function factorialize(num) {
  if (num < 0) {
 return undefined;
  } else if (num === 0) {
    return 1;
  }
  else {
    return num * factorialize(num - 1);
  }
}

The site I'm learning from accepted this solution, but they're only testing for nonnegative integers, i.e for \$n \ge 0\$, not negative integers.

So I got curious on how to compute a negative factorial \$(-n)!\$. Many pages say it's undefined, but I found two saying it could be defined.

Link 1
Link 2

The gist I got is that:

\$|n!| = |(-n)!|\$

Their absolute values are the same but the negative factorials change signs.

Examples:

\$4! = (-4)! = 24\$

\$5! = 120\$ but \$(-5)! = -120\$

The formula that I gathered from the two linked pages is:

\$(-n)! =  |n|! * (-1)^n\$

And this reflects my code. From test cases, I think I've nailed it. I just want to ask if there's a better way of coding it. Someone from here
remarked that using recursion is memory-inefficient.

function factorialize(num) {
 if (num === 0) {
   return 1;
  } else if (num > 0) {
     return num * factorialize(num - 1);
  } else {
     return Math.pow(-1, num) * Math.abs(num) * factorialize(Math.abs(num) - 1);
    }
  }

// test cases below
    console.log(factorialize(-1)); // -1
    console.log(factorialize(1)); // 1
    console.log(factorialize(0)); // 1
    console.log(factorialize(-2)); // 2
    console.log(factorialize(2)); // 2
    console.log(factorialize(-3)); // -6
    console.log(factorialize(3)); // 6
    console.log(factorialize(-4)); // 24
    console.log(factorialize(4)); // 24
    console.log(factorialize(-5)); // -120
    console.log(factorialize(5)); // 120

Changing your function to something like this would work. I don't think you need to model your function after the mathematical equation.

function factorial(num) {
  if (num < 0) {
    return num * factorial(num + 1);
  } else if (num == 0) {
    return 1;
  } else {
    return num * factorial(num - 1);
  }
}

Another solution that uses tail call optimization could be

function factorial(num) {
  function recur(n, acc) {
    if (n < 0) {
      return recur(n + 1, n * acc);
    } else if (n == 0) {
      return acc;
    } else {
      return recur(n - 1, n * acc);
    }
  }
  return recur(num, 1);
}


Find indices of first pair of numbers that sums to zero Here is the task from a beginner's course:

  Write a method that takes an array of numbers. If a pair of numbers in the array sums to zero, return the positions of those two numbers. If no pair of numbers sums to zero, return nil.

This model solution does not use idiomatic Ruby, because at that point in the course, we had only learned traditional loops. I'm trying to go back now and understand how to use idiomatic Ruby to accomplish the same tasks.

Model solution:

def two_sum(nums)

idx1 = 0

while idx1 < nums.length

idx2 = idx1 + 1

while idx2 < nums.length

  if nums[idx1] + nums[idx2] == 0

    return [idx1, idx2]

  end

  idx2 += 1

end

idx1 += 1

end

return nil

end

My solution:

def two_sum(nums)

answer = []

nums.each_with_index do |num1, idx1|
    nums.each_with_index {|num2, idx2| answer.push(idx1, idx2) if (num1 + num2) == 0 }
end

    answer.empty? ? nil : answer[0..1]

end

The call of only the first two items in the answer array seems cheap. There must be a better way. I'm just not sure how to work through the array including the index, but also breaking as soon as the array hits a length of two, or however else one might define it.

The problem can be solved in less than quadratic time. Sort the elements by absolute value, then search for consecutive elements that sum up to zero.

def two_sum(nums)
    temp = nums.map.with_index { |x, i| [x, i] }
    temp.sort_by! { |x| x[0].abs }
    for x in temp.zip(temp.rotate)
        if x[0][0] + x[1][0] == 0
            return [x[0][1], x[1][1]]
        end
    end
    return nil
end


Resize Images Dynamically I set out to make a simple dynamic resize method for images. Obviously, it's better to serve images that are pre-sized to avoid unnecessary server loads. But it was fun to make for a novice/beginner jQuery project and not really meant for a production project. For many of you, I'm sure it'll be rudimentary at best, but that's why I'm posting it here, to get some advice on ways to improve it.

The goal,

Add wrappers dynamically (using the wrapInner method for content structure inside the collection of elements with class topic), add wrapper around images to constrain the content image sizes.
dynamic width & height attributes on images - onLoad & onResize
Dynamic query string on img.src (URL adds ?width=###&height=###)
onLoad & resize (for demo purposes I'm using "placehold.it/###x###") this is
only needed for content images in the DOM.
Real width & height of image set to parent (figure) as max-width/height
onLoad
Debounce/throttle resize events to avoid excess events begin fired. (used a method displayed on css-tricks.com and can be found here)

The CSS and HTML is more to make the demo look nice but if you have some input for that as well, by all means. Like I said, I'm certainly not a jQuery developer, though I'm trying to learn as quickly as possible to augment my skill set. So if you have any advice or input on areas that can improve, I'd greatly appreciate it.

Fiddle: DEMO

$(function() {
    // demo fun
    $('.topic').each(function() {
        var $this = $(this),
            ifMedia = $this.find('.media');
        $this.wrapInner('<section class="content"/>');

        if (ifMedia.length) {
            $this.addClass('media-support');
        }
        if ($this.hasClass('media-support')) {
            $('.media-support:odd').addClass('inverse');
        }
    });
});

var sizeTimer;
$(window).on('load', function() {
    var img = new Image();
    $('img').each(function() {
        var $this = $(this),
            image = new Image(),
            _imgWidth = $(this).width(),
            _imgHeight = $(this).height(),
            _naturalWidth = this.naturalWidth,
            _naturalHeight = this.naturalHeight;
        image.src = $(this).attr("src");
        $this.attr({
            'width': +_imgWidth,
            'height': +_imgHeight,
            'src': '//placehold.it/' + _imgWidth + 'x' + _imgHeight
        }).wrap('<figure class="image" />').closest('.image').css({
            'max-width': _naturalWidth,
            'max-height': _naturalHeight
        });
    });
}).on('resize', function() {
    clearTimeout(sizeTimer);
    sizeTimer = setTimeout(function() {
        $('img').each(function() {
            var $this = $(this),
                _imgWidth = $(this).width(),
                _imgHeight = $(this).height();
            $this.attr({
                'width': _imgWidth,
                'height': _imgHeight,
                'src': '//placehold.it/' + _imgWidth + 'x' + _imgHeight
            });
        });
    }, 250);
});

I know that this code was posted more than three years ago and you have likely learned many things since then but I'd rather not see it be a zombie any longer... so here we go:

In general the code looks okay. There aren't too many repeated blocks. There are a few suggestions I have to tidy up the code.

Cache DOM references - instead of querying for collections like $('img') multiple times, do that once the DOM is loaded, store the collection in a variable and then reference the variable instead of re-querying.

var images
$(window).load(function() {

    images = $('img');
    images.each(function () {

and later in the resize callback:

.on('resize', function () {

    clearTimeout(sizeTimer);

    sizeTimer = setTimeout(function () {

        images.each(function () {

unused variable: img this variable doesn't appear to be used anywhere after it is created: var img = new Image();. Perhaps that is left from a side venture in the code...
Don't repeat code this may only be achievable with advanced function manipulation so I wouldn't expect you to do this in one of your first projects but the code that exists in the callback function on the resize event executed for each image could be abstracted out to a separate function:

function setAttributesOnImage() {

    var $this = $(this),
        _imgWidth = $(this).width(),
        _imgHeight = $(this).height();

    $this.attr({
        'width' : _imgWidth,
        'height': _imgHeight,
        'src': '//placehold.it/' + _imgWidth + 'x' + _imgHeight
    });

}

Then that function can be used in the load callback, utilizing Function.prototype.apply():

image.src = $(this).attr("src");
setAttributesOnImage.apply(this);
$this.wrap('
').closest('.image').css({

    'max-width': _naturalWidth,
    'max-height': _naturalHeight

});

This means _imgWidth and _imgHeight don't have to be declared again in the load handler, since the call to setAttributesOnImage() handles that.

And also in the resize callback:

  .on('resize', function () {

    clearTimeout(sizeTimer);

    sizeTimer = setTimeout(function () {

        images.each(setAttributesOnImage)
    }, 250);

Also, that last timeout callback could be simplified, using Function.prototype.bind() to create a Partially applied function:

sizeTimer = setTimeout(images.each.bind(images, setAttributesOnImage), 250);

Snippet

Expand the snippet below to see simplified code.

/* Goal:

 1. add wrapper around .topic contents (wrapInner) onDocReady
 2. add wrapper around images (wrap) onDocReady

 3. dynamic width & height attributes on images (attr) onLoad & resize
 4. dynamic queryString on img.src (URL adds ?width=###&height=###) onLoad & resize (for demo purposes I'm using "placehold.it") this is only needed for content images in the DOM.

 5. real width & height of image set to parent as max-width/height onLoad
 6. debounce/throttle resize events

*/

$(function () {

    $('.topic').each(function () {

        var $this = $(this),
            ifMedia = $this.find('.media');

        $this.wrapInner('
');

        if (ifMedia.length) {
            $this.addClass('media-support');
        }
        if ($this.hasClass('media-support')){
            $('.media-support:odd').addClass('inverse');
        }
    });

});
function setAttributesOnImage() {

  var $this = $(this),
      _imgWidth = $(this).width(),
      _imgHeight = $(this).height();

  $this.attr({
    'width' : _imgWidth,
    'height': _imgHeight,
    'src': '//placehold.it/' + _imgWidth + 'x' + _imgHeight
  });

    }
var sizeTimer;
var images;
$(window).on('load', function () {
  images = $('img');

    images.each(function () {

        var $this = $(this),
      image = new Image(),
            _naturalWidth = this.naturalWidth,
            _naturalHeight = this.naturalHeight;

        image.src = $(this).attr("src");
        setAttributesOnImage.apply(this);
        $this.wrap('
').closest('.image').css({

            'max-width': _naturalWidth,
            'max-height': _naturalHeight

        });

    });

}).on('resize', function () {

    clearTimeout(sizeTimer);

    sizeTimer = setTimeout(images.each.bind(images, setAttributesOnImage), 250);

});
* {
    box-sizing:border-box;
}
html{background-color:#ecf0f1;}
body{max-width:1200px;color:#34495e;}
.content{max-width:800px;}
body,.content{margin:0 auto;}
h2{font-size:2rem;margin-bottom:15px;line-height:1;}
p{font-family:sans-serif;line-height:1.55}

img {
    display:block;
    max-width:100%;
    height:auto;
}
.topic:nth-of-type(odd) {
    background-color:#cde
}
.topic:nth-of-type(even) {
    background-color:white
}
.media{
    position:absolute;
    transform:translateX(-100%)
}
.content{padding:35px 25px}
@media (min-width:540px){
    .content {
        height:350px;
        padding:0;
    }
    .content:after {
        clear:both;
        content:'';
        display:table;
    }
    .image {
        background-position:center center;
        background-size:cover;
        background-repeat:no-repeat;
        width:100%;
        padding-bottom:30%;
    }
    .copy{
        padding-left:25px;
        padding-right:25px;
        position:relative;
        top:50%;
        transform:translateY(-50%);
    }
    .media-support .copy {
        width:50%;
        float:left;
    }
    .media-support.inverse .copy {float:right}

    .media-support .media {
        width:50%;
        transform:translateX(0);
        float:right;
        height:100%;
        position:relative;
    }
    .media-support.inverse .media {float:left}
    figure {
        margin:0;
    }
    .media .image, .media img, .media div {
        position:absolute;
        bottom:0;
    }
    .media-support:not(.inverse) .image,
    .media-support:not(.inverse) img{
        left:0
    }
    .media-support.inverse .image,
    .media-support.inverse img{
        right:0;
    }
    .media .undock {
        position:absolute;
        bottom:50%;
        transform:translateY(50%);
    }
    .media-support .media .image > img {
        width:100%;
        height:auto;
    }
}

This is a section heading

Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Donec odio. Quisque volutpat mattis eros. Nullam malesuada erat ut turpis. Suspendisse urna nibh, viverra non, semper suscipit, posuere a, pede.

This is a section heading

Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Donec odio. Quisque volutpat mattis eros. Nullam malesuada erat ut turpis. Suspendisse urna nibh, viverra non, semper suscipit, posuere a, pede.

This is a section heading

Donec nec justo eget felis facilisis fermentum. Aliquam porttitor mauris sit amet orci. Aenean dignissim pellentesque felis.

Morbi in sem quis dui placerat ornare. Pellentesque odio nisi, euismod in, pharetra a, ultricies in, diam. Sed arcu. Cras consequat.

&nbsp;


Segmented Sieve Spoj I tried(trying...) to solve the SPOJ prime number problem PRIME1 so i learned and implemented Segmented Sieve correctly but this program works in time for 10^8 but getting Time limit exceeded (TLE) for 10^9 input integer. can someone help me reduce complexity within my code please.

/*
 followed this tutorial
 https://medium.com/@agilanbtdw/prime-number-generation-in-java-using-segmented-sieve-of-eratosthenes-187af1dcd051
*/
#include <iostream>
#include <math.h>
#include <vector>
#include <fstream>
using std::cin;
using std::ifstream;
using std::vector;
using std::cout;
using std::ios_base;
int main() {
ios_base::sync_with_stdio(false);cin.tie(NULL);cout.tie(NULL);
    //ifstream in("inp.txt");
  short int t;
  unsigned long int n ,q;
  cin >> t;
  while(t--){
  cin >>q >> n;
  vector<bool> a(n,true);
  unsigned long int m = sqrt(n);
  a[0] = false;
  a[1] = false;
  // find primes till sqrt
  for(unsigned long int i = 2 ; i <= m ; i++ ){
    if(a[i]){
      for(unsigned long int j = i*i ; j <= m ; j+=i ){
        a[j] = false;
      }
    }
  }
//  store the primes upto m
  vector<unsigned long int> primes;
  for(unsigned long int i = 2 ; i <= m ; i++ ){
      if(a[i]){ primes.push_back(i);}
  }

  unsigned long int temp;
  for (unsigned long int x : primes) {
        temp = q/x;
        temp*=x;
                // from primes arrays increment each prime with that num , set the index as false
        for(unsigned long int y = temp ; y <=n ; y+=x){
        a[y] = false;
        }
  }
// set the falsed indexes in previous primes arrays to true
  for(unsigned long int i = 0 ; i < primes.size() ;++i){
    a[primes[i]] = true;
  }
  for(unsigned long int i =q ; i <= n ; i++ ){
    if(a[i]) {cout << i << " ";}
  }
  cout << "\n";
  }
  return 0;
}

  my repl link - https://repl.it/repls/UnrulyMotherlyTransformation

Reducing Complexity
The first thing to do for reduced complexity is to break up the code into multiple functions. This would allow you to profile the code and see where it spends the most time. This also makes it easier to read, write and debug the code. There are possibly 5 functions in main. As programs get more complex it is generally better to limit the main() function to set up and clean up and do all other processing in sub functions.

This would be applying the Single Responsibility Principle to the code which is the first pillar of SOLID object oriented programming. The Single Responsibility Principle states The single responsibility principle is a computer programming principle that states that every module, class, or function[1] should have responsibility over a single part of the functionality provided by the software ...

Variable Names
Generally single character variable names make code harder to read and debug. It is unclear what the variables t, n, q or m are or do.

Use Explict Casts
The line of code

unsigned long int m = sqrt(n);

is an implicit cast and generates warning messages in some compilers. C++ attempts to be type safe and casts should be explict, in this particular case it would be better to use a static_case.

Performance
It looks like this code checks all numbers in a range, there is no reason to process even numbers except for 2 since all even numbers greater than 2 can't be prime numbers because they are divisible by 2.

Indentation
The code is improperly indented immediately after the while statement. This makes it much harder to read the code.


Calculating ordered differences between elements in vector Input
I start with a vector of integers or floats, e.g.:
[10, 100, 20, 5, 50]

Output
I want to return a list of differences or "deltas" between elements of the sorted version of this vector. Differences are then presented in the same order as the items from the original, unsorted vector.
The above example vector would return, for instance:
[5, 50, 10, 5, 30]

Procedure
The procedure I have is to:

Order the input vector, e.g.,:

[5, 10, 20, 50, 100]

Calculate the differences between each pair of elements in the sorted vector (using the first element as-is; i.e., the difference between that value and zero):

[5, 5, 10, 30, 50]

Reorder differences by the original index of elements from the input vector:

[5, 50, 10, 5, 30]

Function
I am working in Javascript. This is the code I have written so far:
/**
 * Remap input vector into a vector of state-sorted delta scores
 *
 * @param {Array} 1d array of numbers
 * @returns {Array} 1d array of per-element differences sorted by input index indices
 */
remapVectorToIndexSortedDiffs(vector) {
  const l = vector.length;
  const indices = [...Array(l).keys()];
  const vals = indices
    .sort((a, b) => vector[a] < vector[b] ? -1 : vector[a] > vector[b] ? 1 : 0 )
    .map((d, i) => { const k = indices[i]; const v = vector[k]; return { i:k , d:v }; });
  const deltas = new Array(l);
  deltas[0] = vals[0];
  for (let idx = 1; idx < l; idx++) deltas[idx] = { i : vals[idx].i, d : vals[idx].d - vals[idx - 1].d };
  return deltas
    .sort((a, b) => (a.i > b.i) ? 1 : -1)
    .map((d) => d.d);
}

I'm working with a large 2D matrix of such vectors and would like to optimize this function as much as possible, for both memory and speed (though speed is higher priority).
Are there ways I can improve this function in Javascript?

Array is not a Vector
JavaScript does not have vectors, it uses  Array and  Object that can both behave like vectors (grows and shrinks).

Arrays can also be sparse [hash map] or dense [flat sequential array].

Objects are sparse.

Objects do not have a length property

As you have not defined what you mean by vector one must make an assumption in regard to what the variable vector will contain and the general rule for arrays is that they are dense with numeric 0 based indexes.
My assumption that it is an array is based on the following

Is vector an Array?
The question's examples show arrays [5, 10, 20, 50, 100] and Objects do not (may not) have a length property.

Is vector a dense array?
The line  = vector.length; would return the last index of a sparse array. eg a = []; a[100] = 1; console.log(a.length); outputs 101 but has only 1 item. Your code would fail if you passed a sparse array.

Once we have established that the input is a standard array we can begin to optimize the function
Too complex
The first part of the function is overly complex and mostly redundant.
Sort
Array.sort is required but can be simplified as follows
.sort((a,b) => vector[a] - vector[b])

Tracking sorted position
There is no need to track the sort position by creating an object for each item. The sort position is already available in the indices array by index. Eg indices[0] gives the sort position for the first item.
Thus the need to call Array.map and create {i,d} for each item is not needed, however if it were required it could be better
Better the map
Properties names are very poor and the map function is also too complex. the line

.map((d, i) => { const k = indices[i]; const v = vector[k]; return { i: indices[i] , value: vector[indices[i]] }; });

could be be simplified to
.map(pos => ({pos , value: vector[pos]}));

Avoid duplicated processing
You create the array indices using Array.keys . const indices = [...Array(l).keys()] to get an ordered list of indexes, then lower down you create the array const deltas = new Array(l);
Note that the both lines are based on identical arrays. If you create deltas  first and use it to get the keys you can remove one of the created arrays.
  const deltas = new Array(l);
  const indices = [...deltas.keys()];

Calculate deltas and return
There are a lot of changes before we get to the meat of the function so all the rest of your original code can be replaced while considering the following...
Changes

The for loop can be replaced with the simpler while loop.
There is no need to create yet another object for each value. Once you have used the sorted positions they can be ignored.
The final sort and map operations are not required.
The first entry in the resulting array deltasmay not be at position 0 so the arraydeltas` must be pre-assigned space to avoid creating a sparse array.

BTW: Always delimit code blocks, eg for(i = 0; i < 10; i++) foo += i; is better as for(i = 0; i < 10; i++) { foo+=i; }
Rewrite
Changing the overly long function name remapVectorToIndexSortedDiffs with sortedDiffs
The functions complexity (time and storage) is the same,
however its memory use and performance have been improved significantly.
function sortedDiffs(vector) {
      var i = 0;
      const length = vector.length, deltas = Array(length);
      const idxs = [...deltas.keys()].sort((a, b) => vector[a] - vector[b]);
      deltas[idxs[i]] = vector[idxs[i]];
      while (i++ < length - 1) {
          deltas[idxs[i]] = vector[idxs[i]] - vector[idxs[i - 1]];
      }
      return deltas;
}

or a less verbose version.
function sortedDiffs(vecs) {
      var i = 0;
      const len = vecs.length - 1, res = Array(len + 1);
      const idxs = [...res.keys()].sort((a, b) => vecs[a] - vecs[b]);
      res[idxs[i]] = vecs[idxs[i]];
      while (i++ < len) { res[idxs[i]] = vecs[idxs[i]] - vecs[idxs[i - 1]] }
      return res;
}


C++ exception handling I'm getting throw exception and need your review
The main function bellow trying to allocate memory several times and then throw the exception on the upper level.
#include <iostream>
#include <memory>

struct MiserlinessClass{
    char  *   pMemory;
    int       memory_len;
    const int max_size = 10;
    MiserlinessClass(int len){
        if (len>max_size){
            std::cout<<"What a lavish lifestyle! Get out of my face! \n";
            std::bad_alloc exception;
            throw exception;
        }
        pMemory = (char *)malloc(len*sizeof(char));
        memory_len = len;
    }
};

int main(int argc, char** argv){
    int len = (argc==2)? strtol(argv[1],NULL,10): 5;

    std::unique_ptr<MiserlinessClass> objPtr;

    bool allocated = false;
    const int max_cnt = 5;
    int cnt = 0;
    while (!allocated){
        try{
            std::cout<<"Trying to allocate "<<len<<" chars...\n";
            objPtr.reset(new MiserlinessClass(len));
            allocated = true;
        } catch (std::bad_alloc &e){
            len = len >> 1;
            cnt++;
            if (cnt==max_cnt){
                std::cout<<"I give up \n";
                throw  e;
            }
        }
    }
    std::cout<< "Allocated " << objPtr->memory_len << " chars \n";
    return 0;
}

And here are the results of 3 different runs
1 -  allocates memory from the very first attempt,
2 -  tries few times and allocates available and T
3 -  failed after N attempts and throw an exception to the upper level
---------------------------------------
$ make; ./01_exception_pointer 3
make: Nothing to be done for 'all'.
Trying to allocate 3 chars...
Allocated 3 chars
---------------------------------------
$ make; ./01_exception_pointer 64
make: Nothing to be done for 'all'.
Trying to allocate 64 chars...
What a lavish lifestyle! Get out of my face!
Trying to allocate 32 chars...
What a lavish lifestyle! Get out of my face!
Trying to allocate 16 chars...
What a lavish lifestyle! Get out of my face!
Trying to allocate 8 chars...
Allocated 8 chars
---------------------------------------
$ make; ./01_exception_pointer 1023
make: Nothing to be done for 'all'.
Trying to allocate 1023 chars...
What a lavish lifestyle! Get out of my face!
Trying to allocate 511 chars...
What a lavish lifestyle! Get out of my face!
Trying to allocate 255 chars...
What a lavish lifestyle! Get out of my face!
Trying to allocate 127 chars...
What a lavish lifestyle! Get out of my face!
Trying to allocate 63 chars...
What a lavish lifestyle! Get out of my face!
I give up
terminate called after throwing an instance of 'std::bad_alloc'
  what():  std::bad_alloc
Aborted (core dumped)

My questions are

I know the formal difference between returning value and throwing the exception, but still can not feel whether it's time to panic and throw an exception or the program should keep calm and just return the error code.

Is is a proper way to handle bad_alloc exception?

Any extra comments and suggestions :)

I know the formal difference between returning value and throwing the exception, but still can not feel whether it's time to panic and throw an exception or the program should keep calm and just return the error code.

Simple Rules of thumb.

If you can not fix the issue locally then throw an exception.
You can not solve the problem of not having enough memory locally so throw an exception. This will allow the stack to unwind memory be released and you "May" get to a point where this can be solved (or if not let the application exit). An example of where it can be stopped is when you are creating independent task and one of these tasks fails. It does not mean that all tasks will fail. Log the fact that this task has failed allow the exception to release all the memory it used and then try the next task.

Error code should not cross interface boundaries.
Error codes are great if you check them. It allows a simple mechanism to pass information back one or two levels without complicating the code. So if you are writing a library and internally you use error codes that is fine (because you will be good and check all error codes). But you can not trust users of your library so if an error is going to propagate outside your library use an exception to force the user to explicitly get it.

User Input and streams. Don't use exceptions. User input is always going to be error prone and the code that handles user input is going to need to have lots of validation checks (if done correctly). They know this. So on stream operations simply set the stream to bad.

Is a proper way to handle bad_alloc exception?

Let it propagate to the top of your app. Log something to let the user know it happened. If this is an independent task drop the task and start the next one. If this is just a normal part of executing then let the application exit.

Any extra comments and suggestions :)

Don't use malloc()/free() in C++ code.
For every new there should be a delete.
Prefer to use make_unique() rather than new (to help with new/delete matching).

Code Review:
// Owned pointers are a bad idea.
struct MiserlinessClass{
    char  *   pMemory;          // This is an owned pointer.
                                // At lot of extra work needs to be done here
                                // You need to look up the rule of three/five

            std::bad_alloc exception;
            throw exception;

Easier to simply write:
             throw std::bad_alloc;

This is not a bad_alloc situation.
        if (len>max_size){
            std::cout<<"What a lavish lifestyle! Get out of my face! \n";
            std::bad_alloc exception;
            throw exception;
        }

bad_alloc means that the system failed to allocate memory because of memory pressure.
You should use: std::range_error the input parameter was out of range.

Don't use malloc use new here.
        pMemory = (char *)malloc(len*sizeof(char));

        // Better:
        pMemory = new char[len];

Note: You still need to implement the rule of three here.

Note: Yes you should be throwing an exception above. You do not want to allow the user to create invalid objects. This needs to be fixed before the application is allowed into production so forcing the unit test to fail with an exception is the correct solution.

There is no destructor to the class MiserlinessClass. So the memory allocated with malloc() is going to be leaked. Forcing you to run out of memory quicker.
See Rule of three.

Note: This will release the previous object. But only if the new object is successfully created. So you have a bunch of memory allocated. Then you try and allocate twice as much. If that allocation works then you release the old memory.
            objPtr.reset(new MiserlinessClass(len));

I would move the declaration:
   std::unique_ptr<MiserlinessClass> objPtr;

into the try block do the allocation there.
    try{
        std::cout<<"Trying to allocate "<<len<<" chars...\n";
        std::unique_ptr<MiserlinessClass> objPtr = std::make_unique<MiserlinessClass>(len);
        allocated = true;
    } catch (std::bad_alloc &e){
    ...

Now. The memory is allocated and then released at the end of the try. Next time around the loop you know that you have clean store to allocate from as everything was cleaned up from your last attempt.
You could go a step further and simply remove the unique_ptr.
    try{
        std::cout<<"Trying to allocate "<<len<<" chars...\n";
        MiserlinessClass       objPtr(len);
        allocated = true;
    } catch (std::bad_alloc &e){
    ...

Why not just multiply by 2.
            len = len >> 1;

Or is that divide by 2. Either way the intent is not clear. Use code that expresses your intent clearly.

Only prints if there is no exception.
    std::cout<< "Allocated " << objPtr->memory_len << " chars \n";


Find palindromic substrings as efficiently as possible I've written a function to calculate all the distinct substrings of a given string, that are palindromes. The string is passed as the argument to the function. For example, the string abba is a palindrome because reversed it is also abba. If the function takes the string aabaa it returns 5, because the distinct substrings are a, aa, aabaa, aba, b.

I've currently made use of two for loops for this to work, however, when the string gets big, it's very inefficient due to the fact it iterates one by one. Any suggestions as to how I can make this significantly more efficient? Recursion maybe?

The function:

function countPalindromesInString(s) {

    let subStrings = [];

    for (let i = 0; i < s.length; i++) {
      for(let j = 0; j < s.length - i; j++) {
        let subString = s.substring(j, j + i + 1);
        if(subString === subString.split('').reverse().join('') && !subStrings.includes(subString)) {
          subStrings.push(subString);
        }
      }
    }
    return subStrings.length;
}

You code looks pretty neat, i do not think that using recursion would improve it very much, but i have some minor suggestions;

1) Performance-wise improvements

The only thing that seems odd at first glance is how you reverse the string to compare subString.split('').reverse().join('');

I had to look it up, but i found that the way that you are doing its the right way using In-built functions, and its pretty much good for almost all cases, but if you want to improve a little the performance you could use something like:

function reverse(s) {
  var o = [];
  for (var i = 0, len = s.length; i <= len; i++)
    o.push(s.charAt(len - i));
   return o.join('');
}

Using this function is has an improve in performance over your current implementation.

2) Readability

You could extract your conditional and use it as a function so your intent would be more clear:

let isPalindrome = function (word, words) {
  return word === word.split('').reverse().join('') && !words.includes(word)
}

and use this function in the conditional as:

if(isPalisdrome(subString, subStrings)) {
          subStrings.push(subString);
 }

3) Conclusion

In the end putting all together the code would be something like:

function reverse(s) {
  var o = [];
  for (var i = 0, len = s.length; i <= len; i++)
   o.push(s.charAt(len - i));
   return o.join('');
}

function isPalindrome(word, words) {
  return word === reverse(word) && !words.includes(word)
}

function countPalindromesInString(s) {
    let subStrings = [];
    for (let i = 0; i < s.length; i++) {
      for(let j = 0; j < s.length - i; j++) {
        let subString = s.substring(j, j + i + 1);
        if(isPalindrome(subString, subStrings)) {
          subStrings.push(subString);
        }
    }
  }
  return subStrings.length;
}


Haskell insertAt solutions I was working on Haskell Problem 21:

  Insert an element at a given position into a list.

  Example:

* (insert-at 'alfa '(a b c d) 2)
(A ALFA B C D)

I have two different solutions:

--Problem 21 (also not tail recursive)
insertAt :: a -> [a] -> Int -> [a]
insertAt v [] n = [v]
insertAt v arr 1 = (v:arr)
insertAt v (x:xs) n = (x:(insertAt v xs $ n - 1))

--Tail recursive version
insertAtB :: a -> [a] -> Int -> [a]
insertAtB v arr n = loop v n arr []
    where loop v n [] acc = (myReverse acc) ++ [v]
          loop v 1 arr acc = (myReverse acc) ++ [v] ++ arr
          loop v n (x:xs) acc = loop v (n - 1) xs (x:acc)

Which is better stylistically? Which is better in terms of runtime and memory use?

How can you determine what order of arguments is the best for currying? Is it only based on your own usage or is there a better way to determine this?

The first solution is infinitely better than the second solution.  In Haskell, tail recursion is not the main concern.  Rather, the primary consideration is laziness.  Furthermore, operations like list reversal (myReverse, which you haven't shown us) and list concatenation (the ++ operator) should be avoided wherever possible, since they involve traversing an entire list to its end.  Considering that lists in Haskell may be infinitely long, you could be inviting trouble.

I consider your insertAt v [] n = [v] case to be superfluous and harmful.  If you are inserting an element into an empty array, I think it's fair to require the index to be 1.  I would write it like this:

insertAt :: a -> [a] -> Int -> [a]
insertAt v xs     1 = v : xs
insertAt v (x:xs) n = x : (insertAt v xs (n - 1))

In my opinion, the interface specified in the challenge is weird: the function would be better named insertBefore, since I expect that inserting an element at position 0 would place it at the head of the resulting list.

As a rule of thumb, Haskell functions should be designed so that the parameters are ordered starting from the one that is least likely to vary.  I would certainly not place the index at the end.  One better design would be insertAt value index list, such that insertAt value index could be treated as a partial function to be applied to some list.  Another reasonable design would be insertAt list index value, such that insertAt list index could be treated as a partial function that is waiting to fill a specified hole in the list.  However, with the current specification, the partial function insertAt value list means "add value to list, but we don't know where yet", which seems like a rare use case to me.


Linear search algorithm template implementation This template class is a linear search algorithm with a simple search function. I have tested it with int and char and it seems to be working fine. I would like pointers on how I can make it more efficient and on my coding technique. I do know that the standard library provides this algorithm, i am just doing it for fun and practice

sLinear.h

//Linear search template

#ifndef SLINEAR_H
#define SLINEAR_H

namespace fm
{
    template <typename T>
    class sLinear
    {
        T *_list;
        T _item;
        int _sizeOfList;

    public:
        sLinear();
        sLinear(T *myArray, T item, int size);

        ~sLinear();

        int findItem();

    };

    //------public methods------
    template <typename T>
    sLinear<T>::sLinear()
    {
        _list = nullptr;
        _sizeOfList = 0;
    }

    template <typename T>
    sLinear<T>::sLinear(T *myArray, T item, int size)
    {
        _list = myArray;
        _item = item;
        _sizeOfList = size;
    }

    template <typename T>
    sLinear<T>::~sLinear()
    {
        _list = nullptr;
        _sizeOfList = 0;
    }

    template <typename T>
    int sLinear<T>::findItem()
    {
        while (_list != nullptr)
        {
            for (int i = 0; i < _sizeOfList; i++)
            {
                if (_list[i] == _item)
                {
                    return i + 1;
                }

            }

            return -1;

        }
    }
}
#endif

main.h

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

using namespace fm;

int main()
{

    /* Linear search with integer */
    int iList[] = { 1,9,2,6,5,3,7,4,8,0 };
    int size = (sizeof(iList) / sizeof((iList[0])));

    sLinear<int> mySearch(iList, 8, size);
    int k = mySearch.findItem();

    if (k == -1)
        std::cout << "Item not found!" << std::endl;
    else
        std::cout << "Item found:" << k << std::endl;

    return 0;
}

Returning int is dubious; it unnecessarily narrows the range of possible return values. Consider retuning an iterator.
The line if (_list[i] == _item) implies that _list must provide an operator[](int), which is very restrictive. A linear search is just like its name implies, linear. It is expected to work on any linear collection, e.g. forward iterator (maybe even on input iterator).
The class sLinear exposes one public method, and keeps no state. There's no reason to make it a class. A standalone

template <typename I>
I findItem(I first, I last, I::value_type value) {
    ....
}

will do as well.
while (_list != nullptr) is very strange. _list never changes; why do you want to loop?

PS: Could you explain a rationale for returning i+1?


Desktop Notifications for Queue Items - Updated Code A while ago I took one of @Simon's scripts and updated it to make it more geared towards normal users (no offense mods) and this Question was created. But the link to the Github is no longer correct and the code has changed quite a bit to get it to work on Firefox and Chrome (and Opera and IceDragon and Safari).

I would like to know if my code is following UserScript and JavaScript standards and if there is anything that I can do to improve the efficiency of the code itself.

Here is the current Next version

Notification.requestPermission();

var KEY_NEXT = 'NextReload';
var DELAY =  120 * 1000; //120,000 milliseconds = 2 Minutes
var currentTime = Date.now ? Date.now() : new Date().getTime();
var lastTime = GM_getValue(KEY_NEXT, 0);
var nextTime = currentTime + DELAY;
GM_setValue(KEY_NEXT, nextTime);

var timeDiff = Math.abs(lastTime - currentTime);
setTimeout(function(){
    window.location.reload();
}, DELAY);

var notificationTitle = (document.title.split(' - ')[1] + ' Review Queue').replace(' Stack Exchange', '.SE');

// a way to detect that the script is being executed because of an automatic script reload, not by the user.
if (timeDiff <= DELAY * 2) {
    var reviewCount = 0;
    var reviewItems = document.getElementsByClassName('dashboard-num');

    for (var i = 0; i < reviewItems.length; i++){
        if (reviewItems[i].parentNode.className != 'dashboard-count dashboard-faded'){
            reviewCount += parseInt((reviewItems[i].getAttribute("title")).replace(',', ''), 10);
            console.log(reviewItems[i]);
        }
    }
    console.log(reviewCount);

    if (reviewCount > 0) {
        var details = {
            body: reviewCount + ' Review Items',
            icon: 'https://github.com/malachi26/ReviewQueueNotifier/raw/master/Resources/Icon2.jpg'
        }
        var n = new Notification(notificationTitle, details);
        n.onclick = function(){
            window.focus();
            this.cancel();
        }
        setTimeout(n.close.bind(n), 100000); // Magic number is time to notification disappear
    }
}

You can follow development or participate in development at Github as well.

Review Queue Notifier

Haven't actually tested the code, just writing JavaScript from the top of my head. Here's some notable changes:

Using what I call "return early". That way, it avoids nested ifs and your code looks flat.
Renamed a few variables so that they have more meaning, especially DELAY which is very generic and because you have 2 timeouts. Long names don't matter, readability should always be considered.
Instead of using Math.abs, why not reverse the equation? Current time is always larger than time in the past. You'll always get a positive value.
Replaced your loop with reduce because it was designed for that, an aggregator of values.
You can always use the unary + as a shorthand for converting strings into numbers. In most cases, it acts similar to parseFloat. If the string is potentially not a number, guard yourself with isNan.
Replaced your currentTime to just Date.now(). By now, browsers should at least be ES5-compliant. Anything less... you still rolling with IE6 or sumthin'?
Notification title moved down nearer to notification and after the two early-return if. That way, you don't actually generate the title if the script bails out on either of the two returns.
Inlined your notification details object. Assigning to a variable just took up space, only to be used once.

And the end result:

Notification.requestPermission();

var KEY_NEXT = 'NextReload';
var NOTIFICATION_AUTODISMISS_TIMEOUT = 100000;
var RELOAD_DELAY =  120 * 1000; //120,000 milliseconds = 2 Minutes

var currentTime = Date.now();
var lastTime = GM_getValue(KEY_NEXT, 0);
var timeDiff = currentTime - lastTime;
var nextTime = currentTime + RELOAD_DELAY;

GM_setValue(KEY_NEXT, nextTime);

setTimeout(window.location.reload, RELOAD_DELAY);

if (timeDiff > RELOAD_DELAY * 2) return;

var reviewItems = document.getElementsByClassName('dashboard-num');
var reviewItemsArray = Array.prototype.slice.call(reviewItems);
var reviewCount = reviewItemsArray.reduce(function(count, reviewItem){
    return +reviewItem.getAttribute('title').replace(',', '') + count;
}, 0);

if (reviewCount <= 0)  return;

var notificationTitle = (document.title.split(' - ')[1] + ' Review Queue').replace(' Stack Exchange', '.SE');
var n = new Notification(notificationTitle, {
    body: reviewCount + ' Review Items',
    icon: 'https://github.com/malachi26/ReviewQueueNotifier/raw/master/Resources/Icon2.jpg'
});

n.onclick = function(){
    window.focus();
    this.cancel();
};

setTimeout(n.close.bind(n), NOTIFICATION_AUTODISMISS_TIMEOUT);


Using Statement for OleDB - Is this overkill? I am still trying to wrap my head around good programming practice and just wrote this routine to read in an Excel file and return all the sheets into a dataset:

Public Shared Function ReadExcelIntoDataSet(ByVal FileName As String, Optional ByVal TopRowHeaders As Boolean = False) As DataSet
    Try
        Dim retval As New DataSet

        Dim strConnString As String = "Provider=Microsoft.ACE.OLEDB.12.0;Data Source=" & _
                                        FileName & ";Extended Properties=""Excel 12.0;IMEX=1;" & _
                                        "HDR=" & If(TopRowHeaders, "Yes", "No") & """;"

        Using oleExcelConnection = New OleDb.OleDbConnection(strConnString)
            oleExcelConnection.Open()

            Using oleExcelCommandString = New OleDb.OleDbCommand("", oleExcelConnection)

                Using oleExcelAdapter = New OleDb.OleDbDataAdapter(oleExcelCommandString)
                    Dim dtSchema As DataTable = oleExcelConnection.GetOleDbSchemaTable(OleDb.OleDbSchemaGuid.Tables, Nothing)
                    For Each dr As DataRow In dtSchema.Rows
                        Dim Sheetname As String = dr.Item("TABLE_NAME").ToString

                        If Sheetname.EndsWith("$") Then
                            oleExcelCommandString.CommandText = "Select * From [" & Sheetname & "]"
                            oleExcelAdapter.Fill(retval, Sheetname)
                        End If
                    Next

                    Return retval

                End Using

            End Using

        End Using

    Catch ex As Exception
        Dim ErrorData As String = "Unable to Read in Data from the Excel File " & FileName & vbCrLf & _
                                   "Module: Utility > ReadExcelIntoDataSet"
        Throw New Exception(ErrorData & vbCrLf & "ERROR Data:" & vbCrLf & ex.Message)
    End Try
End Function

My first questions are:

Is the use of the Using statements good or is it some kind of over-kill and should this maybe be handled in a Finally statement in stead?
Is this the best method to read in an Excel File (Given that they can be either .xls or .xlsx) in the fastest possible way?
What is the best way to pass the error up to the calling code? Did my Catch statement do that well enough??

The Using is good practice and not overkill. Trying to dispose the resources in the Finally part has the disadvantage, that you don't know when the exception happened and which resources have been assigned by that time. The Using statement does this automatically for you internally: If resouce <> Nothing Then resource.Dispose() End If
You need different connection strings for the different Excel types. You can use this function:

Private Function GetConnectionString(ByVal filename As String) As String
    Const xlsConnString As String = "Provider=Microsoft.Jet.OLEDB.4.0;Data Source={0};Extended Properties=Excel 8.0;"
    Const xlsxConnString As String = "Provider=Microsoft.ACE.OLEDB.12.0;Data Source={0};Extended Properties=Excel 12.0;"

    If Path.GetExtension(filename).Length = 5 Then ' .xlsx, .xltx
        Return String.Format(xlsxConnString, filename)
    Else ' .xls, .xlt
        Return String.Format(xlsConnString, filename)
    End If
End Function

You can either propagate the exception or return Nothing. In my opinion, your version does it very well.


C++ concurrent queue and testing I've written concurrent queue based on std::queue.

#include <queue>
#include <shared_mutex>
#include <condition_variable>

template <typename T, typename S = typename std::queue<T>::container_type>
class concurrent_queue {
    // no std::shared_mutex yet
    using mutex_type = std::shared_timed_mutex;

public:
    explicit concurrent_queue(const S &sequence)
            : queue_(sequence) { }

    explicit concurrent_queue(S &&sequence = S())
            : queue_(std::move(sequence)) { }

    concurrent_queue(const concurrent_queue &other)
    {
        std::shared_lock<mutex_type> other_lock(other.mutex_);
        queue_ = other.queue_;
    }

    concurrent_queue(concurrent_queue &&other)
    {
        std::unique_lock<mutex_type> lock(mutex_, std::defer_lock);
        std::shared_lock<mutex_type> other_lock(other.mutex_, std::defer_lock);
        std::lock(lock, other_lock);
        queue_ = std::move(other.queue_);
    }

    concurrent_queue &operator=(const concurrent_queue &other)
    {
        if (this != &other) {
            std::unique_lock<mutex_type> lock(mutex_, std::defer_lock);
            std::shared_lock<mutex_type> other_lock(other.mutex_, std::defer_lock);
            std::lock(lock, other_lock);
            queue_ = other.queue_;
        }
        return *this;
    }

    concurrent_queue &operator=(concurrent_queue &&other)
    {
        if (this != &other) { // not needed?
            std::unique_lock<mutex_type> lock(mutex_, std::defer_lock);
            std::shared_lock<mutex_type> other_lock(other.mutex_, std::defer_lock);
            std::lock(lock, other_lock);
            queue_ = std::move(other.queue_);
        }
        return *this;
    }

    ~concurrent_queue() = default;

    T pop()
    {
        std::unique_lock<mutex_type> lock(mutex_);
        condition_variable_.wait(lock, [this] { return !queue_.empty(); });
        T item = queue_.front(); // use auto?
        queue_.pop();
        return item;
    }

    bool pop(T &item)
    {
        std::unique_lock<mutex_type> lock(mutex_);
        if (queue_.empty()) {
            return false;
        } else {
            item = queue_.front();
            queue_.pop();
            return true;
        }
    }

    void push(const T &item)
    {
        std::unique_lock<mutex_type> lock(mutex_);
        queue_.push(item);
        lock.unlock();
        condition_variable_.notify_one();
    }

    void push(T &&item)
    {
        std::unique_lock<mutex_type> lock(mutex_);
        queue_.push(std::move(item));
        lock.unlock();
        condition_variable_.notify_one();
    }

    template <typename ...Args>
    void emplace(Args &&... args)
    {
        std::unique_lock<mutex_type> lock(mutex_);
        queue_.emplace(std::forward<Args>(args)...);
        lock.unlock();
        condition_variable_.notify_one();
    }

private:
    mutable mutex_type mutex_;
    std::condition_variable_any condition_variable_;
    std::queue<T, S> queue_;
};

Things I'd like to know about this code:

Possible deadlocks - it seems to be working fine in my program, but I'm only using one producer and one consumer. (see #3)
Exception safety - I guess pop methods can throw, but I don't see how this could be avoided without hacks like external locking (lock-front-pop-unlock)
Testing - I've done some really basic tests, things like multiple readers and writers. It has never crashed but I don't really know how to test it.
Anything that comes to your mind.

The two pop() functions have very different semantics, this is probably more of a taste issue, but i'd normally like to see such a difference in a difference in function name. Yes the signature is different, but there isn't really anything that would tell a user that one is blocking and the other one isn't.

With the copy and move operations, it might help if you thought about the semantics, when does one usually copy an object, when is it moved. Does the expectation that a user has from a copy constructor or the move constructor match what you are doing. It sounds like with the move operator you have some different functionality in mind. If that is that case I'd normally put that in a separate function. Also think of how this queue should be used, is a move or a copy a valid operation.


Java program to add two numbers represented by linked lists First of all, I found this question on Leetcode.The two digits of the two numbers are stored in reverse order.We have to add them and create a single linked list composed of the sum of the two numbers.Here is the question for reference.

public class LLAdd {
    static Node head, head1, head2;

    static class Node {
        Node link;
        int data;

        Node(int data){
            this.data = data;
            this.link = null;
        }
    }

public static void printList(Node head){
    Node curr = head;
    while(curr!=null){
        System.out.print(curr.data + "->");
        curr = curr.link;
    }
    System.out.print("NULL \n");
}

public static Node push(int data, Node head){
    Node new_node = new Node(data);
    new_node.link = head;
    head = new_node;
    return head;
}

public static Node addNumbers(){
    Node curr1 = head1, curr2 = head2;

    if(curr1==null) {
        return curr2;
    }
    if(curr2==null) {
        return curr1;
    }

    int data = 0, units = 0, tens = 0;

    while(curr1!=null || curr2!=null){
        data = (curr1!=null ? curr1.data : 0) + (curr2!=null ? curr2.data : 0) + tens;
        units = data % 10;
        tens = data / 10;
        head = push(units, head);
        curr1 = curr1!=null ? curr1.link : null;
        curr2 = curr2!=null ? curr2.link : null;
        if(curr1==null && curr2==null && tens > 0){
            head = push(tens, head);
        }
    }
    return head;
}

public static void main(String[] args){

    head1 = push(3, head1);
    head1 = push(4, head1);
    head1 = push(2, head1);

    head2 = push(4, head2);
    head2 = push(6, head2);
    head2 = push(5, head2);

    System.out.println("Lists before addition: ");
    printList(head1);
    printList(head2);

    System.out.println("List after addition : ");
    head = addNumbers();
    printList(head);
   }
}

Please review this code and suggest better ways to solve this problem.Thank you.

Advice 1

Fix your code indentation. Instead of

public class LLAdd {
...
public static void printList(Node head){
...
}
...
}

you should have

public class LLAd {
    ...
    public static void printList(Node head){
        ...
    }
    ...
}

Advice 2

In method definitions, one, according to common Java coding conventions, must have a single space between parameter-closing ) and a block-opening {. Instead of

public static void printList(Node head){
    ...
}

you should write

public static void printList(Node head) {
    ...                                ^-- Space!
}

Advice 3

The class name LLAdd is really poor. Taking a look of what it does, you could come up with the name, say, BigInteger.

Advice 4

static Node head, head1, head2;

This is an anti-pattern since there is only one "instance" of the class. Make these non-static and private instead.

Advice 5

Node new_node ...

Once again, Java coding conventions dictate that the fields/variables are named in CamelCase:

Node newNode ...

Advice 6

static class Node {
    Node link;
    int data;

    Node(int data){
        this.data = data;
        this.link = null;
    }
}

Make this private and final since most likely you don't want to shine into the entire package, and you hardly need to derive from that class. Also, I would rename link to next. Finally for this advice, you don't need this.link = null;. Java sets all the object fields to null by default.

Advice 7 In principle, I think you should make your BigInteger/LLAdd immutable.

Advice 8

public static Node push(int data, Node head)...

That's is a poor name too. Looking from what it does, I suggest you rename it to prependDigit. Also, I believe you would want to check that data is within the range 0-9 unless you want to allow dealing with arbitrary radices.

Advice 9

Instead of implementing printList, I would override toString.

Alternative implementation

import java.util.Scanner;

public final class BigInteger {

    private static final class Digit {
        Digit next;
        int digit;

        Digit(char ch) {
            this.digit = ch - '0';
        }

        Digit(int digit) {
            this.digit = digit;
        }
    }

    private Digit leastSignificantDigit;

    public BigInteger(String integerText) {
        checkCharacters(integerText);
        leastSignificantDigit =
                new Digit(integerText.charAt(integerText.length() - 1));
        Digit head = leastSignificantDigit;

        for (int i = integerText.length() - 2; i >= 0; i--) {
            Digit digit = new Digit(integerText.charAt(i));
            head.next = digit;
            head = digit;
        }
    }

    private BigInteger() {

    }

    public BigInteger add(BigInteger other) {
        Digit digit1 = leastSignificantDigit;
        Digit digit2 = other.leastSignificantDigit;
        BigInteger result = new BigInteger();
        Digit headDigit = null;
        boolean carry = false;

        while (digit1 != null || digit2 != null) {
            int intDigit1 = digit1 == null ? 0 : digit1.digit;
            int intDigit2 = digit2 == null ? 0 : digit2.digit;
            int sum = intDigit1 + intDigit2 + (carry ? 1 : 0);
            carry = sum > 9;
            int currentDigit = carry ? (sum - 10) : sum;
            Digit digit = new Digit(currentDigit);

            if (result.leastSignificantDigit == null) {
                result.leastSignificantDigit = digit;
                headDigit = digit;
            } else {
                headDigit.next = digit;
                headDigit = digit;
            }

            if (digit1 != null) {
                digit1 = digit1.next;
            }

            if (digit2 != null) {
                digit2 = digit2.next;
            }
        }

        if (carry) {
            headDigit.next = new Digit(1);
        }

        return result;
    }

    @Override
    public String toString() {
        StringBuilder stringBuilder = new StringBuilder();

        for (Digit digit = leastSignificantDigit;
                digit != null;
                digit = digit.next) {
            stringBuilder.append((char)('0' + digit.digit));
        }

        return stringBuilder.reverse().toString();
    }

    private void checkCharacters(String integerText) {
        for (char ch : integerText.toCharArray()) {
            if (!Character.isDigit(ch)) {
                throw new IllegalArgumentException(
                        "Character '" + ch + "' is not a valid digit.");
            }
        }
    }

    public static void main(String[] args) {
        Scanner scanner = new Scanner(System.in);
        BigInteger bi1 = new BigInteger(scanner.nextLine());
        BigInteger bi2 = new BigInteger(scanner.nextLine());
        System.out.println(bi1 + " + " + bi2 + " = " + bi1.add(bi2));
    }
}


JavaScript closure: code trivia to keep the event handler separate Note: This was originally posted at SO, and as per the recommendation moving to this form.

In my effort to learn JS on the backdrop of SVG development, I got lot of help from SO so far. The answer to previous query was the best to understand closure; its usage and caveats in very short time. While continuing on the same line, I was trying to breakdown the below function so that it is more readable and easy to expand. It was part of an answer given by @Phrogz. I could not think a way to keep the onmove closure outside the loop.

If someone can help me with a pseudo code/skeleton of how can it be done without too many variable to retain the state (like button click state while moving etc), it would clarify many of my doubts regarding how the closure can be assigned etc. You may also break the function into 3 event handlers (mousedown, mousemove, mouseup). I could not think a way to do this while keeping the context (mousemove only triggers when the mouse is down, as it is within the closure).

I use jQuery typically in my projects, so if that helps to make the above code more maintainable, that would help too.

for (var a=svg.querySelectorAll('.drag'),i=0,len=a.length;i<len;++i){
        (function(el){
            var onmove; // make inner closure available for unregistration
            el.addEventListener('mousedown',function(e){
                el.parentNode.appendChild(el); // move to top
                var x = el.tagName=='circle' ? 'cx' : 'x';
                var y = el.tagName=='circle' ? 'cy' : 'y';
                var mouseStart   = cursorPoint(e);
                var elementStart = { x:el[x].animVal.value, y:el[y].animVal.value };
                onmove = function(e){
                    var current = cursorPoint(e);
                    pt.x = current.x - mouseStart.x;
                    pt.y = current.y - mouseStart.y;
                    console.log("pt.x:" + pt.x + "pt.y:" + pt.y);
                    var m = el.getTransformToElement(svg).inverse();
                    m.e = m.f = 0;
                    pt = pt.matrixTransform(m);
                    el.setAttribute(x,elementStart.x+pt.x);
                    el.setAttribute(y,elementStart.y+pt.y);
                    var dragEvent = document.createEvent("Event");
                    dragEvent.initEvent("dragged", true, true);
                    el.dispatchEvent(dragEvent);
                };
                document.body.addEventListener('mousemove',onmove,false);
            },false);
            document.body.addEventListener('mouseup',function(){
                document.body.removeEventListener('mousemove',onmove,false);
            },false);
        })(a[i]);
    }

Add more spaces.
Use more named functions. (wrap the whole thing in an anonimous wrapper if you're worried about name space conflicts).
When you do use lamdas give them names (it makes the code easier to read and you don't get  showing up in the error log.
Declare variables in higher functions to take advantage of closures. eg x, y, elementStart and mouseStart.
camelCase.
Use === unless you explicitly want to force casting.
Don't make functions in a loop.

http://codr.cc/s/03c0d0af/js


Craps game rules and code: version 2 I've taken all the advice from here and have created a Craps class and gameplay.  I just wanted to ask for more advice on how to optimize my code.

#include <iostream>
#include <cmath>    // atan(), fabs()
#include "Craps.h"

using namespace std;

int main(int argc, char* argv[])
{
    int choice;
    Craps game;
    do {
        cin>> choice;
        if(!cin)
        {
            cin.clear();
            cin.ignore(200,'\n');
            cout<<"\n please inter int number \n\n";
        }

        switch (choice) {
            case 1:
                game.Play();
                break;
             case 2:
                break;

            default:
                break;
        }

    } while (choice != 2);

    return 0;
}

#include <iostream>
//#include <catdlib>
#include <ctime>
#include <string>
using namespace std;
class Craps
{
public:
    void DisplayInstruction();
    void Play();
    int DiceRoll();
private:
    int random;
};

void Craps::DisplayInstruction(){
    cout<<"DisplayInstruction"<<endl;
}

int Craps::DiceRoll()
{
    int die1  = (rand() + time(0)) % 6+1 ;
    int die2  = (rand() + time(0)) % 6+1 ;

    int sum  = die1 + die2;
    return sum;

}

void Craps::Play()
{
    bool won = false;
    bool lost =  false;

    string anyKey;

    int myPoint = 0;
    int sumeOfDice = DiceRoll();

    switch (sumeOfDice)
    {
        case 7:
            won= true;
            break;
        case 11:
            won= true;
            break;
        case 2:
            lost = true;
            break;
        case 3:
            lost = true;
            break;
        case 12:
            lost = true;
        break;
            default:
                myPoint = sumeOfDice;
                cout<< myPoint <<endl;
                break;
    }

    while (won  == false && lost ==false) {
        sumeOfDice = DiceRoll();
        if(sumeOfDice == myPoint)
        {
            won = true;
        }
        else
            if(sumeOfDice == 7)
            {
                lost = true;
            }
    }
    if (won ==  true)
    {
        cout<<"player win"<<endl;
    }
    else
    {
       cout<<"player lose "<<endl;
    }
}

#endif /* defined(__Craps__Craps__) */

Try to refrain from using using namespace std, especially in a program like this.  If you were to put it in the header, you could introduce bugs due to issues such as name-clashing.
Your comments next to <cmath> suggests that you're using it, but I don't see these functions used anywhere.  No need to include this library.
#include headers before libraries to prevent unwanted dependency:

#include "Craps.h" // header won't be forced to use <iostream>
#include <iostream>

Naming convention states that functions should start lowercase:

thisIsAFunction();
this_is_also_a_function();

DisplayInstruction() is a needless function as it only displays one line of text.  Functions shouldn't be created just for that.  In general, functions that don't do anything with the data members are best as free functions (non-member functions).
The dice functionality should have its own class, allowing the Craps class to maintain object(s) of it as needed.  This will also ensure separation of concerns.  You may refer to my implementation to get an idea for this.
You didn't seed std::rand() with std::srand().  This will cause rand() to produce the same random numbers each time.  Put this at the top of main() only:

// this requires <cstdlib> and <ctime>
// use nullptr instead if using C++11
std::srand(static_cast<unsigned int>(std::time(NULL)));

However, if you're using C++11, you should no longer use rand.  I'll quote one of my answers:

  You should refrain from using rand altogether in C++11 as it is considered harmful.  Instead, utilize C++11's <random>
  library.

  You also no longer need <ctime> and should use C++11's
  <chrono>.  From this library, you can obtain a seed for a PRNG
  with this:

auto chrono_seed = std::chrono::system_clock::now().time_since_epoch().count();

The switch in Play() could be its own function, specifically of type bool.  As it isn't working with a data member, it can be a free function.  If you go this route, then each case will need a return true or return false.  The default will also need to handle errors resulting from an invalid case value, such as by throwing an exception.
You never use anyKey anywhere, so remove it.
Tested conditions could be shortened like this:

if (condition)  // if (condition == true)
if (!condition) // if (condition == false)

Tested streams can be shortened like this:

if (!(cin >> choice)) // cin >> choice;
                      // if (!cin)


Program for finding Fibonacci primes I think it could have been designed with more object orientation. I don't like how one of my methods calls another from within the method, but I wasn't sure how to return the result because it is a loop.

Also is it ok to have all methods static, or should I be instantiating the classes?

If there is any other improvements please let me know, but these were my main concerns.

App.java:

public class App {

    public static void main(String[] args) {
        long startTime = System.currentTimeMillis();

        Fibonacci.fibonacci();

        long endTime = System.currentTimeMillis();
        long totalTime = endTime - startTime;

        System.out.println("****************************************\n");
        System.out.println("Total Running Time: " + totalTime + "ms");
    }
}

Fibonacci.class:

public class Fibonacci {

    static void fibonacci() {
        long i = 0;
        long j = 1;
        long k = 0;
        while (i < 1E17) {
            k = i + j;
            i = j;
            j = k;
            Prime.prime(k);
        }
    }
}

Primes.java:

public class Prime {

    static void prime(long number) {

        boolean isPrime = false;

        long end = (long) (Math.sqrt(number) + 1);
        for (long i = 2; i <= end; i++) {
            if (number % i == 0) {
                isPrime = false;
                break;
            } else {
                isPrime = true;
            }
        }
        if (isPrime) {
            System.out.println(number);
        }
    }
}

Object Oriented

I wouldn't call your code object oriented. And yes, using static in too many places can be a hint that you are not using OOP correctly.

But your program is so small and specific that this isn't really a bad thing. If you actually have some extension in mind, a different approach might be better, but right now, I would leave it as it is.

But for example, lets say you plan to write a program in the future which prints every odd Fibonacci number, or every Fibonacci number dividable by 3. With your code, this might be harder to do.

If your approach was like this:

public class Fibonacci {
    static void fibonacci(NumberCheck numberCheck) {
        [...]
        // instead of Prime.prime(k); call:
        if (numberCheck.meetsContition(k)) {print(k)}
    }
}

interface NumberCheck {
    boolean meetsContition(int number);
}

public class Prime implements NumberCheck {
    @Override
    boolean meetsContition(int number) {
        // check if number is prime, return true or false
    }
}

public class Odd implements NumberCheck {
    @Override
    boolean meetsContition(int number) {
        // check if number is odd, return true or false
    }
}

It would be easily extendable. But as I said, if you are not planning on extending your code, you don't really need OOP in this case. I wouldn't even create the Fibonacci and Prime classes, they are more confusing than they are helpful, just put the methods in App.

Naming

prime should better be called isPrime, and I would find it easier to read if it then returned true or false (move the printing to fibonacci).

Other

I would pass an argument to fibonacci for up to which number it should run. Just hardcoding 1E17 is bad style.


Bottom up merge sort in Haskell Although this sort works on Int lists < length million, when the stack overflows, the last clause of the merge function definition looks odd to me, recursing in 2 places. Is there clearer way to define that definition and how can the stack limitation be avoided?

msort :: Ord a => [a] -> [a]
msort [] = []
msort xs = concat . merge . split $ xs

merge :: Ord a => [[a]] -> [[a]]
merge [] = []
merge [x] = [x]
merge (l1:l2:ls) = merge $ mpair l1 l2 : merge ls

mpair :: Ord a => [a] -> [a] -> [a]
mpair [] l2 = l2
mpair l1 [] = l1
mpair l1@(x:xs) l2@(y:ys) | x >= y     = x : mpair xs l2
                          | otherwise  = y : mpair l1 ys

split :: [a] -> [[a]]
split xs = [[x]| x<-xs]

You've spotted the problem. Your merge function is flawed.  For each pair it processes, it introduces an extra unnecessary call to merge.  That is, for a list like [1,2,3,4,5,6], instead of your first merge call expanding directly to:

merge [[1],[2],[3],[4],[5],[6]]
= merge $ mpair [1] [2] : mpair [3] [4] : mpair [5] [6] : []

it expands to:

merge [[1],[2],[3],[4],[5],[6]]
= merge $ mpair [1] [2] : merge $ mpair [3] [4]
  : merge $ mpair [5] [6] : merge []

As a result, your count of merge calls is O(n) and your count of mpair calls is O(n^2) (or similar -- I didn't check exactly). when they should be O(log n) and O(n log n) respectively.

Instead, you want to split merge up into two functions:

merge :: Ord a => [[a]] -> [[a]]
merge [] = []
merge [x] = [x]
merge ls = merge (mergePairs ls)

mergePairs :: Ord a => [[a]] -> [[a]]
mergePairs (l1:l2:ls) = mpair l1 l2 : mergePairs ls
mergePairs ls = ls

This will speed up the algorithm enormously, and it will now run on tens of millions of integers.


Finite state machine written in OCaml to test for the sequence 0,1 I got inspired by Stack Overflow to build a simple state machine which can test if a sequence contains 0,1. I have two questions:

How can I get rid of line | _ -> (function `q0 -> `q0 | `q2 ->
`q2 | `q1 -> `q1)
How can I improve that code?

(If it is matters, I came from Python and many things in OCaml seem unusual to me).

(*
--->  q0  ---0-->  q2  ---1--->   q1
    /  \         /  \          /    \
    -1->         -0->          -0,1->
*)

type ('state, 'number) state_machine = {
    initial: 'state;
    final: 'state -> string;
    transition: 'number -> 'state -> 'state;
};;

let state_machine_01 sequence =
    let machine_01 = {
        initial = `q0;
        final = (function `q0 -> "Init" | `q2 -> "Found zero" | `q1 -> "END" );
        transition = (function
            | 0 -> (function `q0 -> `q2 | `q2 -> `q2 | `q1 -> `q1)
            | 1 -> (function `q0 -> `q0 | `q2 -> `q1 | `q1 -> `q1)
            | _ -> (function `q0 -> `q0 | `q2 -> `q2 | `q1 -> `q1)
        );
    } in

    let state = ref machine_01.initial in
    for i = 0 to (List.length sequence) - 1 do
        state := (machine_01.transition (List.nth sequence i)) !state
    done;
    machine_01.final !state;
;;

let () =
    Printf.printf "------------- start --------------- \n";

    Printf.printf "%s\n" (state_machine_01 [1; 1; 0; 0; 0; 1; 0; 1]);
    Printf.printf "%s\n" (state_machine_01 [1; 1; 0; 0; 0]);
    Printf.printf "%s\n" (state_machine_01 [1; 1; 1]);

    Printf.printf "-------------- end ---------------- \n";
;;

I notice that you represent states as functions. This does give some expressive power, as seen in the composition example given in your linked answer.

If that power isn't needed we can write things directly and simply:

type element = Z | O

let rec start list = look_for_zero list

and look_for_zero = function
  | [] -> "nothing"
  | Z::rest -> have_zero rest
  | O::rest -> look_for_zero rest

and have_zero = function
  | [] -> "nothing"
  | O::_ -> "yes!"
  | Z::rest -> look_for_zero rest

If you want to stick with the given framework, then you can simplify a bit and transform into a functional style:

type ('state, 'number) state_machine = {
  initial: 'state;
  final: 'state -> string;
  transition: 'state -> 'number -> 'state; (* I changed the order! *)
};;

type state = Init | HaveZero | FoundZeroOne
type element = Zero | One

let final = function
  | Init | HaveZero -> "no"
  | FoundZeroOne -> "yes"

let transition state n =
  match n, state with
  | _, FoundZeroOne -> FoundZeroOne
  | Zero, _ -> HaveZero
  | One, Init -> Init
  | One, HaveZero -> FoundZeroOne

let machine = {
  initial = Init;
  final;
  transition;
}

let run_machine machine list =
  machine.final
    (List.fold_left machine.transition machine.initial list)


Lots of Continues in an Inner Loop and Dictionary of Actions I'm using ABCPDF (Version 8) to update an existing PDF document and I believe its API is hurting me in this situation and was curious whether anyone had any comments on how this looks as it appears ugly to me.

The situation: I am replacing pages in PDF documents that are all generated by an in-house process so I can rely on their format being consistent and the following code is tasked with updating annotation links on the Table of Contents page(s) to point to the replace pages instead of the old pages. Since there can be multiple replacement operations, I decided to put the collection of these link annotations into a dictionary instead of looping through all of them for each replacement operation.

As my comments in the code mention, I only was able to piece this together by debugging a lot and examining the properties of different ABCPDF objects.

Here's the relevant code:

//use less than or equal to here because a one page TOC will have a matching start and end page number
private Dictionary<int, Action<int>> _linkUpdates;
WebSuperGoo.ABCPDF8.Doc pdfDoc;
for (var pageNumber = startPageNumber; pageNumber <= endPageNumber; pageNumber++)
{
    pdfDoc.PageNumber = pageNumber;
    var annotCount = pdfDoc.GetInfoInt(pdfDoc.Page, "Annot Count");

    //this is really ugly code that looks for link annotations (i.e. links in the TOC) that reference our old page Id
    //it depends on some nasty PDF data structures in ABCPDF and assumes they will always appear the same way
    //it took me a few hours of debugging to figure out how to get at this stuff and modify it, not pleasant
    for (var i = 1; i <= annotCount; i++)
    {
        //if at any level we come up empty, just get out of this annotation and try the next one
        var annotId = pdfDoc.GetInfoInt(pdfDoc.Page, string.Format("Annot {0}", i));
        var annotationMaster = (Annotation)pdfDoc.ObjectSoup[annotId];
        if (annotationMaster.SubType != "Link")
        {
            continue;
        }
        var annotItem = annotationMaster.Resolve(Atom.GetItem(annotationMaster.Atom, 0)) as DictAtom;
        if (annotItem == null)
        {
            continue;
        }
        var destination = annotItem["D"] as ArrayAtom;
        if (destination == null)
        {
            continue;
        }
        var destinationPage = destination[0] as RefAtom;
        if (destinationPage == null)
        {
            continue;
        }
        _linkUpdates.Add(destinationPage.ID,
            updatedPageId =>
            {
                destinationPage.ID = updatedPageId;
                Atom.SetItem(destination, 0, destinationPage);
            }
        );
    }
}

I included the declarations of _linkUpdates and doc for reference. So after I have this collection populated, when I have an update to make, I can simply do this:

if (_linkUpdates.ContainsKey(oldPageId))
{
     _linkUpdates[oldPageId](newPageId);
}

I fully understand that this logic is heavily dependent on an understanding of how ABCPDF works, but since there's a fairly active community on SO, I figured this might get some responses, perhaps from somebody who knows of an easier way of getting at/updating these link annotations.

So, to summarize the areas I am interested in feedback on are:

The prevalence of continues in my inner for loop
The choice of using a dictionary with an Action value to update the related ABCPDF object

This does work fine, and I am operating in an environment where I will have at most 70 possible link annotations to consider.

If you do this often, you could write an extension method for object that takes a function, which is executed only when the input is not null. You could then use it in a fluent manner to navigate multiple times:

public static class Navigator
{
    public static TOutput Navigate<TInput, TOutput>(
        this TInput input, Func<TInput, TOutput> navigationFunction)
        where TOutput : class
    {
        if (input == null)
            return null;

        return navigationFunction(input);
    }
}

Usage:

var destinationPage =
    (annotationMaster.Resolve(Atom.GetItem(annotationMaster.Atom, 0)) as DictAtom)
        .Navigate(annotItem => annotItem["D"] as ArrayAtom)
        .Navigate(destination => destination[0] as RefAtom);

if (destinationPage == null)
    continue;

Notice that I also removed the braces. This is a matter of personal opinion (and sometimes company policy), but I think that they're not always necessary, especially when the body of the condition is this simple.