C++ breadth-first search using adjacency matrix

A bug

The path printed by main (1, 3, 4, 6, 7) is not shortest. The culprit is the printPath: it needs to know, for each node \$u\$ the value \$u.\text{parent}\$ from which we entered \$u\$. Needless to say, you have never computed it, and printPath computes some path.

Advice 1

struct Vertex is unnecessary since your nodes are represented via simple int values.

Advice 2

You compute the shortest path distances to each node from the source node, but you never use them; you could remove that functionality.

Advice 3

You don't need the colors either. Actually, you don't need in particular the black color. You can simulate whether the node is white or gray via simple inclusion of the node (effectively an int value) to an unordered_set.

Advice 4

adjMatrix is not the best possible name. Some C++ projects prepend each class member field with m_. A better name (in my opinion) would be a verbose m_adjacency_matrix.

Advice 5

I recommend you do not use adjacency matrices for sparse graphs; you will likely waste a hell lot of space. Use adjacency lists instead.

Advice 6

Also, I would remove the printPath from Graph and implement it as a simple function.

Alternative implementation

Below is the way I would go about reimplementing your program. I don't claim it to be too good C++, but that's a start:

#include <algorithm>
#include <iostream>
#include <queue>
#include <unordered_map>
#include <unordered_set>
#include <utility>
#include <vector>

class Graph
{
    std::vector<std::unordered_set<int>> m_adjacency_list;

public:
    Graph(int size) : m_adjacency_list(size) {}

    std::unordered_map<int, int> BFS(const int);
    void addEdge(const int, const int);
};

std::vector<int> traceback_path(int source,
                                int target,
                                std::unordered_map<int, int>&& parent_map)
{
    std::vector<int> path;
    int current_node = target;

    while (true)
    {
        path.push_back(current_node);

        if (current_node == source)
        {
            break;
        }

        current_node = parent_map[current_node];
    }

    // Reverse the path:
    std::reverse(path.begin(), path.end());
    return path;
}

std::unordered_map<int, int> Graph::BFS(const int source)
{
    std::unordered_map<int, int> parent_map;
    std::unordered_set<int> closed;
    std::queue<int> frontier;

    parent_map[source] = 0;
    frontier.push(source);

    while (!frontier.empty())
    {
        int current_node = frontier.front();
        frontier.pop();

        for (const int neighbor : m_adjacency_list[current_node])
        {
            if (closed.find(neighbor) == closed.cend())
            {
                closed.insert(neighbor);
                parent_map[neighbor] = current_node;
                frontier.push(neighbor);
            }
        }
    }

    return parent_map;
}

void Graph::addEdge(int u, int v)
{
    // Undirected graph:
    m_adjacency_list[u].insert(v);
    m_adjacency_list[v].insert(u);
}

int main()
{
    Graph grp1(8);
    grp1.addEdge(0, 1);
    grp1.addEdge(0, 2);
    grp1.addEdge(1, 3);
    grp1.addEdge(1, 4);
    grp1.addEdge(2, 5);
    grp1.addEdge(3, 4);
    grp1.addEdge(3, 6);
    grp1.addEdge(4, 6);
    grp1.addEdge(4, 7);
    grp1.addEdge(6, 7);

    std::unordered_map<int, int> parent_map = grp1.BFS(0);
    std::vector<int> path = traceback_path(1, 7, std::move(parent_map));

    for (int node : path)
    {
        std::cout << node << "\n";
    }
}


Performance concerns when converting GTK image to greyscale

By commenting out various parts of grayscaleFromRGB, we can deduce that what is taking the most time is getElems (and possibly the computations after return). Indeed, for a 2000x1000 picture, there are 2M pixels (x3 or 4 channels), and getElems is putting them in a list word by word. Lists are quite inefficient for storing large amounts of data like pictures (at least 4 or 5 extra words per element, and the GC comes with an extra x2 factor).

To reduce memory usage, we can write a custom loop to process the elements in the array as we read them, like below. To reduce that further, don't use lists to store large amounts of data. It still takes a while to read millions of elements from memory byte by byte; there may be a better way using a specialized data processing library but I don't know it.

grayscaleFromRGB :: Image -> IO [Word8]
grayscaleFromRGB img = do
    imgType <- img `get` imageStorageType
    if imgType == ImageEmpty
        then
            return (replicate 256 0)
        else do
            pixbuf <- imageGetPixbuf img
            pixels <- (pixbufGetPixels pixbuf :: IO (PixbufData Int Word8))
            nChannels <- pixbufGetNChannels pixbuf
            (_, bound) <- getBounds pixels
            loop pixels nChannels bound 0 []

    where
        -- the returned list will be reversed compared to the original code
        loop pixels n bound i acc | i + 2 > bound = return acc
        loop pixels n bound i acc = do
          let get i = fromIntegral <$> (readArray (pixels :: PixbufData Int Word8) i) :: IO Double
          a <- (0.3 *) <$> get i
          b <- (0.59 *) <$> get (i + 1)
          c <- (0.11 *) <$> get (i + 2)
          loop pixels n bound (i + n) $! (((:) $! round (a + b + c)) $! acc)


Converting multi-dimensional array into row data

As long as this input array is static in its depth and structure, the best and fastest technique (due to lowest overhead) is to use old-school loops.
Beyond that, I would:

Not declare variable that will not be used
Use square brace pushing syntax
Avoid excessively wide lines of code which demand the developer to horizontally scroll
Use words as variable names to make your code more readable

Code: (Demo)
$final = [];
foreach ($categorizedOptions as $categories) {
    foreach ($categories as $categoryId => $category) {
        foreach ($category['attribute'] as $attributeId => $attribute) {
            foreach ($attribute['option'] as $optionId => $option) {
                $final[] = [
                    'category' => $categoryId,
                    'categoryname' => $category['name'],
                    'attribute' => $attributeId,
                    'attributename' => $attribute['name'],
                    'option' => $optionId,
                    'optionname' => $option['name']
                ];
            }
        }
    }
}

Don't worry with trying to convert this code into some sexy one-liner.  Trying to do so with recursion or nested functions will make your code ugly and negatively impact performance.
Even trying to get fancy with "array destructuring" and a compact() call don't look worth the effort.  (Demo)


Submitting form after validations using jQuery

There is no need for submitflag. The validform function could simply return the value of validflag, which should be declared inside that function, not in the outer scope.

  In my form I put all input boxes and select into .valid1 class, all checkbox and radio buttons to .valid2 class. I am just wondering is there any way we can do this by putting all input sources under a single class.

Sure. You just need to adjust some of the dom selectors a little bit, so that they are specific enough to match the right kind of input, for example by adding a condition on the type attribute, to make sure that text input and checkboxes are correctly distinguished.


Flexbox layout, columns, rows, multiple heights

Use a different selector: div.subcontainer

Flexbox can work in either column or row. Because the code has a column and row you need to add another div (div.sub-container) with flexbox and change the height to the height of div.sub-container.


I created a rudimentary pull request to your GitHub repo. I won't show all of the content here except for the main file:

import numpy as np
import itertools as it
from math import factorial
import re

def permutation(m, n):
    return factorial(n) / (factorial(n - m) * factorial(m))

def matrix_combinations(matr, n):
    timed = list(map(list, it.combinations(matr, n)))
    return np.array(list(timed))

def array_combinations(arr, n):
    timed = list(map(list, it.combinations(arr, n)))
    return np.array(list(timed))

def check_extreme(matr, arr, x, sym_comb, m):
    sym_comb = sym_comb.replace(']', '')
    sym_comb = sym_comb.replace('[', '')
    sym_comb = re.split("[ ,]", sym_comb)
    for i in range(int(m)):
        td_answer = sum(matr[i] * x)
        if sym_comb[i] == '>':
            if td_answer <= arr[i]:
                return 0
        elif sym_comb[i] == '>=':
            if td_answer < arr[i]:
                return 0
        elif sym_comb[i] == '<':
            if td_answer >= arr[i]:
                return 0
        elif sym_comb[i] == '<=':
            if td_answer > arr[i]:
                return 0
        elif sym_comb[i] == '=':
            if td_answer != arr[i]:
                return 0
        elif sym_comb[i] == '!=':
            if td_answer == arr[i]:
                return 0
        else:
            return 0
    return 1

def extreme_points(A, b, sym_comb):
    # Input
    A = np.array(A)
    b = np.array(b)
    m, n = A.shape
    # Process
    ans_comb = np.zeros((1, n))
    arr_comb = array_combinations(b, n)
    matr_comb = matrix_combinations(A, n)
    for i in range(int(permutation(n, m))):
        if np.linalg.det(matr_comb[i]) != 0:
            x = np.linalg.solve(np.array(matr_comb[i], dtype='float'),
                                np.array(arr_comb[i], dtype='float'))
            ans_comb = np.vstack([ans_comb, x])
    ans_comb = np.delete(ans_comb, 0, axis=0)
    j = 0
    for i in range(len(ans_comb)):
        if check_extreme(A, b, ans_comb[j], sym_comb, m):
            ans_comb = ans_comb
            j += 1
        else:
            ans_comb = np.delete(ans_comb, j, axis=0)
    # Output
    return ans_comb

Notable changes:

Do a direct import of factorial
Don't call asscalar, since it's both unneeded and deprecated
Don't call a variable all, since that shadows a Python built-in
Don't need to explicitly pass array dimensions, nor do you need to reshape the arrays
Drop redundant parens around some expressions
Use += where applicable
Fix up almost all PEP8 issues, except for your capital letter A, which is fine in context

This doesn't solve the bigger issue that you should replace 99% of this with a call to scipy. I'll do that separately (I suspect that @vnp is, as well).


First of all the controversial point: there are very good chances that you do not need a Repository. Why?

Entity Framework already implements Unif of Work. No need to manually handle anything outside very very special cases.
There are infinitesimal chances that you'll need to switch to another ORM (and if you will do then you will need to rewrite your service layer because EF-specific code pervades those classes).

Honestly: how many times you actually did it? I think I did it three times (in 20 years) and if I have a service layer (and it is well-written) then it's probably the only part you need to rewrite.

Drop all that code and make your code easy to read, programming is already complicate and error-prone without adding useless complexity.

Model

Do you actually need read/write properties? Details vary according to the EF version you're using but at least Image.Id might be read-only.

Validate your inputs! Is an empty file name valid?

public sealed class Image
{
    public int Id { get; }
    public string FileName
    {
        get => _fileName;
        set
        {
            if (String.IsNullOrWhiteSpace(value))
                throw new ArgumentException("...");

            _fileName = value;
        }
    }

    public Image(int id, string fileName)
    {
        _id = id;
        FileName = fileName;
    }

    private readonly int _id;
    private string _fileName;
}

Please do better than this (for example I hate to throw ArgumentException when input is null, ArgumentNullException should be used instead). If file name can be null (in your DB!) then just ignore this. Also note that if not required by your ORM then class should be sealed.

View Model

Same as above, do you need read/write properties? If not then make the relevant ones read-only.

I'm perplex about the ToModel() static method. Is there a good reason you're returning an Expression instead of the object itself? If for any reason you need some kind of lazy creation (and for such simple model without any heavy initialisation I can't imagine the reason) then why you're using an expression instead of a simple Func<T>?

Exists() and IsWebp() are named as properties but declared as methods. If the operation might be expensive then you're right to use methods but then you should name them as verb+noun (for example CheckIfFileExists()). If they're trivial then just use properties:

public bool Exists => !String.IsNullOrWhiteSpace(FileName);

ImageSize is declared here but it's not in the model. Do not post partial code  (or, at least, be sure it compiles).

Service

Asynchronous methods should have the Async suffix. It's not required but it's a well-accepted convention and you should follow it.

InsertAsBatch() is misleading. Which batch? I see just one parameter. It might be InsertCore(), InsertImpl() or simply dropped all together because it does not add any value.

In the Delete() method you're doing something extremely dangerous: you're swallowing an error. If trying to delete a non existing image is a soft error you may, at least, return a flag (it's what, for example, .NET collection classes do). I'd prefer a good old exception but that's just my opinion. Also do not repeat your code, to search an image you already have GetImage() method:

public async Task<bool> DeleteAsync(string fileName)
{
    var image = await GetImage(fileName);
    if (image == null)
        return false;

    this.Remove(image);
    await this.UnityOfWork.SaveAsync();

    return true;
}

Whatever you pick be consistent: some methods silently ignore this issue (like DeleteAsync() while others will throw at run-time (like ImageExists()).

public async Task<bool> ImageExistsAsync(string fileName)
    => (await GetImageAsync(fileName))?.Exists() ?? false;

Validate your parameters! I can always pass null for fileName and I don't get any error. Even more serious: decide if null is valid or not because in GetImageAsync() you're calling ImageModel.FileName.Equals() and it'll undoubtedly fail with a tough NullReferenceException if that property is null.

I'm not sure if GetAllImages() should be asynchronous or not. You're reading values from DB and sometimes it might be useful to filter them ("...all images that contain text ABC...") but forcing materialization (even if asynchronous) precludes this possibility. It's also true that you should introduce a separate service method for this then I'd keep it as-is (just adding async in the name).

Are ImageModel.Exists() and ImageModel.IsWebp() used only inside the service? If yes then they should probably be internal instead of public.

Error handling? I do not see a single line to handle DB errors (for example concurrent updates). Is it everything inside Repository base class?


While the given function does work for simple cases, it will not perform as expected if there are multiple colors within richText. The first color will override all other colors.

First, a look at your implementation:

Why is richText used in some places and the (global?) text in others? It looks like these are expected to be equivalent, so you should just use richText since it is explicitly passed in.
Instead of mutating the global text, you should return the replaced result. This makes testing the function easier and debugging much less painful.
Why is this a class function? It seems to have well defined inputs & outputs that does not depend on the state of any object.
Instead of manually padding numbers with element.length === 1 ? '0' + element : element, you should use element.padStart(2, '0') if possible. The polyfill isn't that large if it isn't supported everywhere you need it yet (IE), and it is considerably easier to read.
parseInt ignores leading whitespace, so there's no need to do str.trim().
String.prototype.replace accepts a replace function, which will be called with each match. Use this to extract the rgb values instead of doing re.exec yourself. In the vast majority of cases, you should not need re.exec.
Should text inside the element really be converted? If you are manipulating HTML as text, you probably shouldn't be. If this rgb() text is in the style element of the element, you could be using element.getAttribute('style') instead.
What should happen if I wrote rgb(1, 2, 3, 4, 5, 'asdf')?

Here is how I would write this function, keeping your assumption that all rgb strings are well formed.

function cleanupRichText(richText) {
  return richText.replace(/rgb\((.+?)\)/ig, (_, rgb) => {
    return '#' + rgb.split(',')
      .map(str => parseInt(str, 10).toString(16).padStart(2, '0'))
      .join('')
  })
}

const tests = [
  ['rgb(234, 112, 30)', '#ea701e'],
  ['rgb(0, 0, 0)', '#000000'],
  ['RGB(255, 255,255)', '#ffffff'],
  ['rgb(0, 0, 0) rgb(170, 170, 170)', '#000000 #aaaaaa']
]

for (const [text, expected] of tests) {
  const result = cleanupRichText(text)
  console.log(result === expected ? 'PASS' : 'FAIL', text, '->', result)
}


Best way to find the largest sorted suffix in a list - USACO Sleepy Cow Sorting Python

Nice solution and implementation. Few suggestions:

Naming: the name main is generally used for the entry point of the program. In this case, a better name could be steps_to_sort_cows.
Sublist bounds: since n is fixed, instead of sublist = nums[i:n] you can write sublist = nums[i:]

Complexity
The time complexity is \$O(n*n*log(n))\$. Basically, for each cow sort all the remaining cows. Typically, it's considered not optimal for large inputs.
The space complexity is also not optimal. For each cow two lists are created, one for the new sublist and one for the sorted sublist.
Improved
To reduce space and time complexity, find the size of the sorted sublist in this way:
def steps_to_sort_cows(n, cows):
  first_sorted_cow = cows[-1]
  sorted_cows = 0
  for c in reversed(cows):
    if c > first_sorted_cow:
      return n - sorted_cows
    first_sorted_cow = c
    sorted_cows += 1
  return 0

Starting from the end, count the number of sorted cows and then return n - sorted_cows.
This approach takes \$O(n)\$ time and \$O(1)\$ space complexity.


my constructor is doing work. Is this a code smell in this example?

Personally the only issue I see with the code in terms of your constructor doing real work or testability is the instantiation of an EventEmitter object. Doing this in the constructor prevents you from mocking out or injecting a different class/instance for testing. You could use Dependency Injection as a simply way to resolve this:

var EventEmitter = require('events').EventEmitter,
    Queue = module.exports = function Queue( emitter ) {
    /*  Constructor is doing work..??
    */
    var _emitter    = this._emitter     = emitter,
        _awaiting   = this._awaiting    = [],
        _queue      = this._queue       = [];

    _emitter.on('offer', function (data) {
        _awaiting.shift()(data);
    });

};


Get average post score of TOP 10 tags on StackOverflow using Stack Exchange Data Explorer

Oh well, I didnt notice the sql-server tag...

What a detailed execution plan they have :)

But anyway the answer may be as simple as that there is more answers than there are questions.

And because you are making another subselect for the column and you are selecting what you already have in the outer select, this may get expensive quite fast.

So I would move the subquery from columns to source tables.

Something like this (sorry, I'm not familiar with sql server syntax):

SELECT TOP(10)
  Tags.Id,
  Tags.TagName,
  COUNT(Questions.Id) AS 'numOfQuestions',
  SUM(Answers.AnswerCount) AS 'numOfAnswers',
  (SUM(Answers.AnswerCount) + 0.0) / COUNT(Questions.Id) AS 'answersPerQuestion',
  (SUM(Questions.Score) + SUM(Answers.ScoreSum) + 0.0) / (COUNT(Questions.Id) + SUM(Answers.AnswerCount)) AS 'averageScore'
FROM Tags
INNER JOIN PostTags
  ON PostTags.TagId = Tags.Id
INNER JOIN Posts Questions
  ON Questions.Id = PostTags.PostId
LEFT JOIN (
  SELECT
    Question.Id AS QuestionId,
    COUNT(Answer.Id) AS AnswerCount,
    Sum(Answer.Score) AS ScoreSum
  FROM Posts Question
  INNER JOIN Posts Answer
    ON Question.Id = Answer.ParentId
  GROUP BY Question.Id
) AS Answers
  ON Answers.QuestionId = Questions.Id
GROUP BY Tags.Id, Tags.TagName
ORDER BY COUNT(Questions.Id) + SUM(Answers.AnswerCount) DESC

Few notes:

I'm intentionally doing left join with the subquery and then inner join in the subquery itself to limit the size of the subquery result. But this means that the outer query must handle possible nulls and replace them with zeroes, which I have not done (because idk sql server syntax), but you should know what I mean.

There is a lot of aggregations going on, but only top 10 are selected in the end. It is possible that the above approach may still not be the most performant. In which case I would recommend to split the query into few separate queries. First extract top 10 using only what is needed for this decision. Then aggregate the other stuff on subset of all posts limited to those 10 tags.

EDIT: I have now noticed that in your question you state:

  The query is ordered by the total number of questions

But your query is not doing that. It orders by COUNT(*) which means that a question without answers is counted as +1 and question with N (N>0) answers is counted as +N.

At first I made my query sort it by number of questions and answers together. Which is yet something different. But to comply with your statement, it should be ORDER BY COUNT(Questions.Id) DESC only. And should have been COUNT(DISTINCT Questions.Id) in your original query...


Checking whether an array contains a value or another array

Objects

One edge case you didn't test is isIn({property: 'value'}, [{property: 'value'}]). I don't know what you want it to do here, but currently it will return false.

You also did not test what happens when no values are passed in, or just a value. While TypeScript may not agree with not passing values to functions that are asking for them, it seems to still be possible.

And another one: JSON.stringify has it's own quirks

If undefined, a Function, or a Symbol is encountered during conversion it is either omitted (when it is found in an object) or censored to null (when it is found in an array). JSON.stringify can also just return undefined when passing in "pure" values like JSON.stringify(function(){}) or JSON.stringify(undefined).
All Symbol-keyed properties will be completely ignored, even when using the replacer function.
The instances of Date implement the toJSON() function by returning a string (the same as date.toISOString()), thus they are treated as strings.
The numbers Infinity and NaN as well as the object null are all considered as null.
For all the other Object instances (including Map, Set, WeakMap and WeakSet), only their enumerable properties will be serialized.


Since the number of rows is to be equal to the number of columns, you can simply get the number of columns from the first input and stop taking input when the user enters the same number of rows:

l = []
while not l or len(l) < len(l[0]):
    l.append(list(map(int, input().split())))

so that given an input of:

2 3 4
1 2 3
5 6 7

l would become:

[[2, 3, 4], [1, 2, 3], [5, 6, 7]]


I threw your code through a profiler and it told me that ToList was responsible for quite a bit of your execution time. So why is that?

You have 2 ToList calls in your code::

var nonRepeated = groups.Where(x => x.freq == 1).ToList();
if (nonRepeated.Count() != 0)
{
    var singles = nonRepeated.Select(x => x.number).ToList();
    var smallest = singles.Min();

So what happens here? You start by iterating through groups to find those with freq equal to 1. Count is optimized to lists, so that one should be constant time. Then you iterate through nonRepeated to select number and then finally you iterate through it to select the minimum value. That's a lot of iterations. Let's try to cut that down:

var nonRepeated = groups.Where(x => x.freq == 1)
                        .Min(x => x.number);
if (nonRepeated != null)
{
    var playerNumber = Array.IndexOf(numbers, nonRepeated);

By comparing the execution time of this and the other one (on my system) it seems we have achieved a small improvement. Not enough to brag about though. Since we don't have any ToList calls anymore, I profiled the code once again. This time it is Min that takes up the time (not really surprising - It is forcing the GroupBy, Select and Where to be evaluated). So our bottleneck is (still) the LINQ queries. Can we speed it up?

  Hint: LINQ usually have a slight overhead compared to doing things 'manually'.

Let's try to do these things ourselves, instead of relying on LINQ. Let's replace that GroupBy and Select stuff:

var values = new Dictionary<string, int>();
foreach (var number in numbers)
{
    if (values.ContainsKey(number)) //No curly-brackets to keep it short.
        values[number]++;
    else
        values.Add(number, 1);
}

A benchmark of the code told me that was a clear improvement. Nice! But the profiler still says that Where and Min are expensive. Let's do those manually too.

string minValue = null;
foreach (var value in values)
{
    if (value.Value == 1)
    {
        if (minValue == null || String.Compare(value.Key, minValue, StringComparison.InvariantCulture) == -1)
            minValue = value.Key;
    }
}

if (minValue != null)
    ...

Once again, it seems we got an overall improvement in performance. Interestingly now the bottlenecks seem to be WriteLine and OpenText. We could try to experiment with other ways of reading from file/writing to console, but I don't really feel like doing that right now (and I don't think the 'easy' methods like File.ReadAllLines and File.ReadLines is much faster than your way, if at all). You could try to play with StringBuilder and only write to console once, but in my experience it is rarely an improvement when the method is only executed once.

But maybe you can do some other things to squeeze just a little bit more out of your method.

First of all, is the IsNullOrEmpty really necessary? It's a safety mechanism and in real life we would probably want to have it. But this is a coding exercise and we are all about performance right now.

Another thing you might want to consider is that strings are expensive. As it is, you are currently working with all your numbers as strings. Split creates a bunch of new strings and every time we compare values we do a string comparison. It might be worth to convert your values to int or char instead.

  HINT: If you try to convert to int values, it can be done by using char ASCII values. You know that the values will always be in the range [1-9]. The ASCII value for '1' is 49, '2' is 50, etc. So if you do value[0] - 48, you can convert you strings to ints with very little overhead.

When you want to optimize something it is always a good idea to measure and profile it and see just where the time is spent. Sometimes you can get big improvements with very few changes.. But you can also make things worse with very few changes.

After the changes mentioned in this post (except the string to int conversion) is applied, the code will look something like this:

using (StreamReader reader = File.OpenText(args[0]))
{
    while (!reader.EndOfStream)
    {
        var linevalue = reader.ReadLine();
        var numbers = linevalue.Split(' ');

        var values = new Dictionary<string, int>();
        string minValue = null;
        foreach (var number in numbers)
        {
            if (values.ContainsKey(number))
                values[number]++;
            else
                values.Add(number, 1);
        }

        foreach (var value in values)
        {
            if (value.Value == 1)
            {
                if (minValue == null || String.Compare(value.Key, minValue, StringComparison.InvariantCulture) == -1)
                    minValue = value.Key;
            }
        }

        if (minValue != null)
        {
            var playerNumber = Array.IndexOf(numbers, minValue);
            Console.WriteLine(playerNumber + 1);
        }
        else
        {
            Console.WriteLine("0");
        }
    }
}

As you can see, you have 2 loops within the while now. It might be possible to merge them into one by doing something clever.

I tried the code on CodeEval and got a 181ms execution time. That is a 74 ms improvement. To motivate you a bit, I know it can be done (at least a little) faster. ;)


def loosing_life? warrior
    @health[-1] < @health[-2] ? true : false
  end

  def next_empty? warrior
    warrior.feel.empty?
  end

  def safe? warrior
    next_empty?(warrior) && !loosing_life?(warrior)
  end

Why I see no reason not to just make these part of the warrior class. It would make calling these much cleaner as you're never passing anything but a warrior into them.

  def life_bar_on_screen
    puts "_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ LIFE #{@health.last}"
    (1..@health.last).each { |x| print "▒ "}
    puts ""
  end

The life bar is a really cool idea. I like that a lot.

  elsif @backview[1].to_s.include?("Captive")
    warrior.walk! :backward
  elsif @backview[0].to_s.include?("Captive")

Anytime the same string literal shows up more than once, I recommend using a constant instead. It will protect you from spelling errors that can cause runtime problems. Apply the same advice to your seek_and_shoot? method.

  def seek_and_shoot?
    foe = [ "Wizard" , "Archer", "Sludge"]
    o, oo, ooo = @view[0].to_s , @view[1].to_s , @view[2].to_s
    case
      when o == "nothing" && foe.include?(oo) then true
      #when o == "nothing" && ranged_foe.include?(oo) then true
      #when o == "nothing" && oo == friend then false
      when o == "nothing" && oo == "nothing" && foe.include?(ooo) then true
      else false
    end
  end

The variable names here are pretty obtuse. What exactly are o, oo, and ooo? They're related somehow, but that's all I can readily gather from the names.

Also, commented out code is dead code. Remove dead code. If you're worried that you'll need it, then I recommend you start using source control of some kind. Personally, I like Git.


process numerical input arguments of mixed ints, floats, and "array-like" things of same length>1

I suggest using isinstance instead of type to check the types of variables. You can read about it in details here: What are the differences between type() and isinstance()?
So, for example, instead of writing:

if type(x) in (int, float, np.float64):

you would write:

if isinstance(x, (int, float)):

You can check that it works for np.exp(1) which is of type np.float64.
When the x is of type range the following check is redundant:

if all([type(q) in (int, float) for q in y])

as the elements of y will be always integers. Also, there is no need to convert range to list. The following will also work:

result = float(x[0]) if len(x) == 1 else np.array(x)

To check if a list is empty in Python we usually write:

if not others:

instead of:

if len(others) == 0:

Imports should be at the top of the script. Move the import numpy as np line there.
In the future, when you have a function that can accept variables of different types and its behavior depends on which type it gets, you could try using singledispatch. I could come up with the following implementation:

from collections.abc import Iterable
from functools import singledispatch
from numbers import Real

import numpy as np

@singledispatch
def fixit(x):
    return None

@fixit.register
def _(x: Real):
    return float(x)

@fixit.register
def _(x: Iterable):
    y = np.array(x)
    if y.dtype in (np.int, np.float) and len(y.shape) == 1:
        return float(y[0]) if len(y) == 1 else y.astype(float)
    else:
        return None

I didn't check it thoroughly but for your test cases it works. (but looks a bit ugly)
A better way to solve your problem would be to convert immediately all the values to NumPy arrays of at least one dimension. We would require a np.atleast_1d function for that:

def to_normalized_data(values):
    arrays = list(map(np.atleast_1d, values))
    sizes = set(map(np.size, arrays))
    has_bad_types = any(array.dtype not in (np.int32, np.float64) for array in arrays)
    if len(sizes) > 2 or has_bad_types:
        return None
    max_size = max(sizes)
    singletons = [float(array[0]) for array in arrays if array.size == 1]
    iterables = [array.astype(float) for array in arrays if array.size == max_size]
    return singletons + iterables

>>> to_normalized_data(all_good_things)
[42.0,
 3.14,
 3.141592653589793,
 2.718281828459045,
 3.0,
 42.0,
 3.0,
 42.0,
 array([1. , 2.3, 2. ]),
 array([3.14, 1.  , 4.  ]),
 array([1., 2., 2.]),
 array([3., 1., 4.]),
 array([0., 1., 2.]),
 array([0., 1., 2.])]
>>> sum(to_normalized_data(all_good_things))
array([149.13987448, 149.29987448, 156.99987448])
>>> print(to_normalized_data(some_bad_things))
None

Answering your comments:

  For some reason my anaconda's numpy (1.17.3) returns int64 rather than
  int32 as you have in item 6

Looks like this behavior is OS-specific. Probably a better way to check the types of the obtained arrays would be by using np.can_cast. So, instead of writing:

has_bad_types = any(array.dtype not in (np.int32, np.float64) for array in arrays)

we could write:

has_bad_types = not all(np.can_cast(array.dtype, np.float64) for array in arrays)

  Item #6 doesn't reject two different lengths if no singletons are present. With ([1, 2, 3], [1, 2, 3, 4]) as input, the output is [array([1., 2., 3., 4.])] and [1, 2, 3] just falls through the cracks and disappears

Welp, I missed this case... We can add it back as:

if len(sizes) > 2 or 1 not in sizes or has_bad_types:

  also my original script tested if len(x.shape) == 1 in order to reject ndm > 1 arrays which item #6 doesn't, but that can be easily added back with something like testing for set(map(np.ndim, arrays)) == set((1,)). This is important because np.size won't distinguish between a length=4 1D array and a 2x2 array.

Yep, that's right. Taking all the above into account, the final code could look like this:

def to_normalized_data(values):
    arrays = list(map(np.atleast_1d, values))
    sizes = set(map(np.size, arrays))
    have_bad_types = not all(np.can_cast(array.dtype, np.float64) for array in arrays)
    have_several_dimensions = set(map(np.ndim, arrays)) > {1}
    if len(sizes) > 2 or 1 not in sizes or have_bad_types or have_several_dimensions:
        return None
    max_size = max(sizes)
    singletons = [float(array[0]) for array in arrays if array.size == 1]
    iterables = [array.astype(float) for array in arrays if array.size == max_size]
    return singletons + iterables


I am going to refactor this in small steps so that you can follow my thought process and see where you agree and you disagree.

First, let's start with some style. In Ruby, self is the implied receiver of a message send if you don't supply one, it is generally not necessary to state it explicitly:

class Numeric
  def reverse
    str = to_s.reverse

    if is_a?(Float)
      return str.to_f
    elsif is_a?(Integer)
      return str.to_i
    end
  end
end

Conditionals are expressions in Ruby, not statements. In fact, everything is an expression in Ruby. So, conditionals evaluate to a value, which means that instead of returning a value from each of the branches, we can return the value of the entire conditional expression instead:

class Numeric
  def reverse
    str = to_s.reverse

    return if is_a?(Float)
      str.to_f
    elsif is_a?(Integer)
      str.to_i
    end
  end
end

Being an expression-oriented language, the value of a method body (and also a block body, module body, class body, any block of code, really) is the value of the last expression evaluated inside that body. There is no need to explicitly return (or next in case of a block) it:

class Numeric
  def reverse
    str = to_s.reverse

    if is_a?(Float)
      str.to_f
    elsif is_a?(Integer)
      str.to_i
    end
  end
end

For multi-way conditionals like this, it is usually more legible to use a case expression, instead of a series of if and elsifs:

class Numeric
  def reverse
    str = to_s.reverse

    case
    when is_a?(Float)
      str.to_f
    when is_a?(Integer)
      str.to_i
    end
  end
end

Now we can start doing some real interesting changes. Module#=== checks whether its argument is an instance of self. Object#is_a? checks whether self is an instance of its argument. IOW: the two methods are mirror images of each other, you can usually exchange o.is_a?(m) with m === o and vice-versa. This allows us to simplify our case expression even more:

class Numeric
  def reverse
    str = to_s.reverse

    case self
    when Float
      str.to_f
    when Integer
      str.to_i
    end
  end
end

Now, on to the big thing!

In an object-oriented language, it is always possible to replace a conditional with runtime polymorphic message dispatch. Message dispatch is more powerful than conditionals. Smalltalk is the existence proof of that, it doesn't even have conditionals built into the language, its conditionals are implemented in the library, using message dispatch, kind of like this:

class TrueClass
  def if_then_else(then_block, else_block)
    then_block.()
  end
end

class FalseClass
  def if_then_else(then_block, else_block)
    else_block.()
  end
end

(2 > 3).if_then_else(-> { puts 'True' }, -> { puts 'False' })
# False

This gives rise to the Replace Conditional with Polymorphism Refactoring. (Here is an example of it in Ruby.)

In your case, you are not just switching behavior based on some abstract notion of "type" of the object, you are literally switching behavior based on the class. Even worse: you are switching based on the class of self.

That is a big honking red flag: self always knows what class it is. It should never ever have to check for its own class!

What you are basically doing is re-implementing message dispatch:

foo.bar

will run different code, depending on the class of foo. That's already built into Ruby, you don't have to re-implement it yourself.

So, we let Ruby take care of making the decision whether to invoke the Float or the Integer version of our code:

class Integer
  def reverse
    str = to_s.reverse

    str.to_i
  end
end

class Float
  def reverse
    str = to_s.reverse

    str.to_f
  end
end

Okay. So, the last bit we have here is a little bit of code duplication, which we can get rid of by using the Extract Method Refactoring followed by the Pull Up Method Refactoring.

Extract:

class Integer
  def reverse
    reverse_to_s.to_i
  end

  private def reverse_to_s
    to_s.reverse
  end
end

class Float
  def reverse
    reverse_to_s.to_f
  end

  private def reverse_to_s
    to_s.reverse
  end
end

Pull Up:

class Numeric
  private def reverse_to_s
    to_s.reverse
  end
end

class Integer
  def reverse
    reverse_to_s.to_i
  end
end

class Float
  def reverse
    reverse_to_s.to_f
  end
end

Now we're done. Our code is nice and clean, all our methods do exactly one thing, there are no IFs or other conditionals.

The only slightly smelly bit left is the monkey-patching we are doing. Refinements to the rescue!

module ReversibleNumeric
  refine Numeric do
    private def reverse_to_s
      to_s.reverse
    end
  end

  refine Integer do
    def reverse
      reverse_to_s.to_i
    end
  end

  refine Float do
    def reverse
      reverse_to_s.to_f
    end
  end
end

314159.reverse
# NoMethodError

using ReversibleNumeric

314159.reverse
# => 951413

[Well, actually, Refinements are still in a state of flux, and the above code may or may not work, depending on your exact version and implementation of Ruby.]


Storing the contents of text fields and inserting the stored values on button-click

Your implementation of Storage lends itself nicely to using a dataclass.
from dataclasses import dataclass

@dataclass
class Storage:
    field1: str = ""
    field2: int = -1
    field3: str = ""

This makes your class definition really concise and readable and provides improved scalability and convenient implementations of built-in functions if you ever need them.
You should also consider giving fields and widgets more meaningful names (maybe even changing button text to match their functionality) to improve readability and usability.
Further material on dataclasses:

Data Classes in Python 3.7+ (Guide) on RealPython
Python dataclasses on Youtube by mCoding

The way you create the three buttons does not scale well, changing this part will have to be done by hand and is therefore error-prone. I'm talking about this code snippet:
btn_1 = QPushButton('Button 1')
btn_2 = QPushButton('Button 2')
btn_3 = QPushButton('Button 3')

for w in [btn_1, btn_2, btn_3]:
    w.setProperty('data', Storage())
    w.setCheckable(True)
    w.clicked.connect(self.set_data)
    self.btn_group.addButton(w)
    h_layout.addWidget(w)

I find the following approach to be a lot cleaner, while producing the same result. You basically only need to change the stop argument in range(start, stop) if you want to change the amount of buttons:
for i in range(start=1, stop=4):
    btn = QPushButton(f"Button {i}")
    btn.setProperty('data', Storage())
    btn.setCheckable(True)
    btn.clicked.connect(self.set_data)

    self.btn_group.addButton(btn)
    h_layout.addWidget(btn)

    setattr(self, f"btn_{i}", btn)

I'm sure there's a better way to set up update_storage and set_data without hardcoding the widgets, but I didn't think of a satisfactory approach yet.


Updating database based on selected check boxes

OMITTING CURLY BRACES ?

Don't do that a single line statements like this are acceptable even tho they are still not recommended you should always have {} :

if (Foo is Bar)
    return "Foo is Bar";
else
    return "Foo is not Bar";

But this is a huge red flag for bad code style :

foreach (var id in guids)
{
    if (!_memberInterestService.IsSubInterestExist(id))
        _memberInterestService.Add(new MemberInterestDto
        {
            UserProfileId = item.Id,
            InterestSubCategoryId = id
        });
    else
        _memberInterestService.Delete(id, item.Id, false);
}
foreach (var id in guids)
    if (_memberInterestService.IsSubInterestExist(id))
        _memberInterestService.Delete(id, item.Id, true);

If your statement needs to be written on few lines You should have {} e.g in your foreach as it's body contains more than a single line, same goes for your if's it's kinda hard to read it that way and follow the flow of your program.

Aside from that I would move dbIds && union inside your if (item.InterestIds != null) as you are performing potentially unnecessary operations, they will be used only if the if is triggered anyway :

if (item.InterestIds != null)
{
    IEnumerable<Guid> dbIds = memberIntersestIds.Select(p => p.InterestSubCategoryId);
    IEnumerable<Guid> union = dbIds.ToList().Union(item.InterestIds);
    Guid[] guids = union as Guid[] ?? union.ToArray();
    //..
}

This line I assume you have because you are avoiding multiple enumeration

Guid[] guids = union as Guid[] ?? union.ToArray();

But why not call .ToArray() on union instead, there is no need to create new variable here, you are always working with guids and never using union itself as IEnumerable<> :

IEnumerable<Guid> dbIds = memberIntersestIds.Select(p => p.InterestSubCategoryId);
IEnumerable<Guid> guids = dbIds.ToList().Union(item.InterestIds).ToArray();

This part of your method :

foreach (var id in guids)
{
    if (_memberInterestService.IsSubInterestExist(id))
    {
        _memberInterestService.Delete(id, item.Id, true);
    }
}

foreach (var id in guids)
{
    if (!_memberInterestService.IsSubInterestExist(id))
    {
        _memberInterestService.Add(new MemberInterestDto
        {
            UserProfileId = item.Id,
            InterestSubCategoryId = id
        });
    }
    else
    {
        _memberInterestService.Delete(id, item.Id, false);
    }
}

It's weird, and so is the .Delete() method. First you have a foreach that remove duplicates from the database

foreach (var id in guids)
{
    if (_memberInterestService.IsSubInterestExist(id))
    {
        _memberInterestService.Delete(id, item.Id, true);
    }
}

But apparently you don't trust your methods and you keep searching if there is any item that exist in your db with a matching ID from the set you've previously iterated and already removed. What's the point ?

But why you deleting the items from the db only the first time ? What's different ? It's the same ID collection.

Since there is no obvious reason why are you doing this, I would suggest to combine those in a single foreach :

foreach (var id in guids)
{
    if (_memberInterestService.IsSubInterestExist(id))
    {
        _memberInterestService.Delete(id, item.Id, true);
    }
    else
    {
        _memberInterestService.Add(new MemberInterestDto
        {
            UserProfileId = item.Id,
            InterestSubCategoryId = id
        });
    }
}

With all of those changes your code should  look like this :

public void Update(UserProfileDto item)
{
    MemberInterestService _memberInterestService = new MemberInterestService();
    IList<MemberInterestDto> memberIntersestIds = _memberInterestService.GetAllPlusDeleted();

    if (item.InterestIds != null)
    {
        IEnumerable<Guid> dbIds = memberIntersestIds.Select(p => p.InterestSubCategoryId);
        IEnumerable<Guid> guids = dbIds.ToList().Union(item.InterestIds).ToArray();
        foreach (var id in guids)
        {
            if (_memberInterestService.IsSubInterestExist(id))
            {
                _memberInterestService.Delete(id, item.Id, true);
            }
            else
            {
                _memberInterestService.Add(new MemberInterestDto
                {
                    UserProfileId = item.Id,
                    InterestSubCategoryId = id
                });
            }
        }
    }
    else
    {
        foreach (var memberInterest in memberIntersestIds)
        {
            _memberInterestService.Delete(memberInterest.InterestSubCategoryId, item.Id, true);
        }
    }
}

Your Delete method might be better of with a .FirstOrDefault() instead  of .Single() unless you really want to verify there is exactly one item, .FirstOrDefault() should be faster in most cases as it will return whenever the first item is found, where .Single() will always be O(n) operation because it needs to verify if there is exactly one occurrence.

Probably more can be improved, but again more context is required.


Trello list scraper that extracts all numbers from boards and logs the sum

It is good to see that you are using the if __name__=='__main__': guard. However, I think you are doing a bit too much in your main and I would make a get_total_month function like this:

...
def get_total_month(month, board_lists):
    costs = 0
    month = month.lower()
    for entry in board_lists:
        if month in entry.name.lower():
            for card in entry.cards:
                costs += float(card.name.split('-')[1])
    return costs

def main():
    board = conn.get_board('BE89pW61')
    total = get_total_month('august', board.lists)
    print 'total is {}'.format(total)

This way it will be easier to incorporate for example something like:

totals_per month = [get_total_month(month, boards.lists) for month in months]

which is then easy to plot e.g. with matplotlib:

import matplotlib.pyplot as plt
...

totals = [get_total_month(month, boards.lists) for month in months]
months = range(len(months))

plt.plot(months, totals)
plt.show()

Adding the actual names of the month (and maybe also their year) is a bit more involved, but you can have a look at the examples here and here.

Regarding whitespace, try to stick to one convention, preferentially the one set by pythons offical style-guie, PEP8. This means in particular not writing sum (total) or float( entry ), but sum(total) and float(entry).


This spec creates very few connects, is that plausible?

When you get lots of instances, it's not really practical to look through them all. What most Alloy users would do in this case is to add a constraint to the show predicate to see if you can get the instance you expect. So I modified it to
pred show {
#connects > 4
#requests > 4
}

and now I see instances with more than 4 connects and requests.
Another suggestion: don't use exactly unless you really need to. You usually want to look for all instances up to some bound, and if you set it to exactly, you might find that you get no instances because it's not possible to construct one with exactly that number. For example, if you don't allow conference calls and you require phones to matched one to one, and you then set the bound to "exactly 3" you would get no solutions.


Compare two lists to see if one is a rotation of the other 2

If you compare one of the lists with all the rotations of the other list, this will take \$\Theta(n^2)\$ in the worst case.

For example, if we make test cases like this:

from timeit import timeit
def test(n):
    l1 = [0] * n
    l2 = [0] * (n - 1) + [1]
    return timeit(lambda:is_rotated(l1, l2), number=1)

then the quadratic runtime can be clearly seen in the timings:

>>> test(10**3)
0.009426131844520569
>>> test(10**4)
0.5701698779594153
>>> test(10**5)
57.70295810396783

There are a couple of ways to solve this problem in linear time:

Search for one list in the concatenation of the other list with itself, using a search algorithm that has linear time in the worst case, for example Knuth–Morris–Pratt.

Here's an illustration of this approach using strings instead of lists:

>>> a, b = 'basketwork', 'workbasket'
>>> a in b + b
True

Find the lexicographically minimal rotation of each of the lists to be compared (for example using Booth's algorithm) and see if the results are the same.


Your code seems to be working fine, but I have some suggestions:

Make sure that no line is longer than 80 characters
Instead of exiting when input is <2 or >20, instead prompt for a new input (your code also works for input 1, but not for 0)
Instead of the function handling the printing make it return the statement string
Avoid having variables with the names of builtins, e.g. max
The last pass doesn't add any functionality

To further improve your solution, I would do one of two things:

Continue with the same logic, but reduce the cluttering

I would definitely change from using while loops to using for loops. With a bit of math, the function can be reduced to:

def create_triangle(n):
    triangle = ""
    for i in range(n):
        triangle += ' '*(2*n-i-1) + '/' + ' _'[i==n-1]*2*i + '\\' + "\n"
    for i in range(n, 2*n):
        triangle += ' '*(2*n-i-1) + '/' + ' _'[i==2*n-1]*(2*i-2*n) + '\\'
        triangle += ' '*(4*n-2*i-2) + '/' + ' _'[i==2*n-1]*(2*i-2*n) + '\\'
        triangle += '\n'
    return triangle

Most of it is similar to what you have in your code, the only new thing being ' _'[i==n-1]*2*i. This statement could be hard to interpret, but the only thing it does is to print _ when i==n-1, and a space otherwise.

Change to a more general approach for these kinds of problems

If you want to solve more complex problems related to creating patterns with strings, I'd instead use an array based solution. A function like this:

def create_print_array(n):
    arr = [['*' for i in range(n)] for j in range(n)]
    return '\n'.join([''.join(row) for row in arr])

returns a pattern like this:

*****
*****
*****
*****
*****

for input n = 5. Since we have a 2d matrix, we can directly manipulate the values to form our desired output. With a bit of math again, the function can be written as:

def create_triangle_array(n):
    # create a 2D array filled with spaces
    arr = [[' ' for i in range(4*n)] for j in range(2*n)]
    # create the outer triangle legs
    for i in range(2*n):
        arr[i][2*n-i-1] = '/'
        arr[i][2*n+i] = '\\'
    # create the inner triangle legs
    for i in range(n, 2*n):
        arr[i][i] = '\\'
        arr[i][4*n-i-1] = '/'
    # create the bases for all three triangles
    for i in range(2*n-2):
        arr[n-1][n+1+i] = '_'
        arr[2*n-1][2*n+1+i] = '_'
        arr[2*n-1][1+i] = '_'
    # join the array into a string
    return '\n'.join([''.join(row) for row in arr])

Both of these approaches give the same form on the output, but it should be noted that the second approach gives some extra spaces on the end of each line, to make each line equally long. Both approaches also work for input >= 0 (I left out error-handling for input < 0 since you already covered that in your code).

As a final note, I'd say that your code looks very good for a beginner. It is easy to follow, and you have covered potential wrong input from the user.


Possible bug

If you were to set a right node to the node r with

r.setRight(new Node("v"));

the result would not contain this node.

OOP paradigm

There are several fundamental issues with your solution. Mainly, it breaks completely the OOP paradigm; this is shown by:

The use of static global variables:

public static Node root = null;
public static Node temp = root;
public static Node anotherTemp;

This is not a good idea: this creates variables that can be seen and modified by anyone, when in reality, they only make sense when flattening the nodes.
The flattening algorithm depends on the value itself!

if (!m.getValue().equals("m")) {

The issue with this line is that you have just made your algorithm dependent on your data. It cannot flatten any node since "m" is hard-coded. If tomorrow your first node has a value of "a", it will be broken.

In Object-Oriented Programming, you should give behaviour to objects. Your object in this case is a Node, and you are trying to flatten it. Therefore, it makes sense to have a method flatten() inside the class Node: any node can be flattened and it is the responsibility of a Node to be able to flatten itself.

Let's reason through what it means for a node n to be flattened:

The new right of node n will be the result of flatting what was below it.
After that, the right-most node of n will have at its right set to the flattened previous right of n.

This shows a simple recursive solution:

class Node {

    // ...

    public Node flatten() {
        Node node = new Node(value);
        Node prevRight = right;
        if (down != null) {
            node.setRight(down.flatten());
        }
        if (prevRight != null) {
            Node rightMost = node;
            while (rightMost.getRight() != null) {
                rightMost = rightMost.getRight();
            }
            rightMost.setRight(prevRight.flatten());
        }
        return node;
    }

}

Be careful of typos also, flattern is mis-spelled: it should be flatten.


You could create a graph, and use graph theory algorithms to find all nodes (children) you can visit from your current node (parent). The graph would be a directed, acyclic graph due to the nature of the file system.

In fact, this sounds like a great example of how to use graph theory in practice, so I will implement it given slightly more time.

I am including a simplified version of your code without using graphs (still had to use a nested loop, but only of depth 2!):

def relationship_dictionary(dataframe, key_column_name, values_column_name):
    """
    The key_column_name is the primary data source that should be considered the
    start of the nested relationship.

    The values_column_name is the subfolder

    Creates a dictionary of unique relationships to each key.
    """
    # ['A', 'B', 'C', 'D', 'E', 'F', 'C', 'C']
    parents = dataframe[key_column_name].to_list()

    # ['B', 'C', 'E', 'E', 'Z', 'Z', 'B', 'A']
    children = dataframe[values_column_name].to_list()

    # [('A', 'B'), ('B', 'C'), ('C', 'E'), ('D', 'E'), ('E', 'Z'), ('F', 'Z'), ('C', 'B'), ('C', 'A')]
    queue = tuple(zip(parents, children))

    # Create a parent -> empty set mapping to avoid using "if parent in mapping then ..., else ..."
    mapping = {parent: set() for parent in parents}

    # Iterate over each parent, child pair
    for parent, child in queue:

        # Always register a new pair has been processed
        mapping[parent].add(child)

        # Need to iterate over current pairs to make sure situations such as
        # 1. Pair A -> {B} added
        # 2. Pair B -> {C} added
        # result in A -> {B, C} instead of A -> {B}
        #
        # This essentially checks that if the parent in the current pair has been a child somewhere, the child in
        # current pair should also be added to wherever the parent was a child (if confusing follow sample above),
        # excluding cases (such as the last C -> {A} pair being included into A -> {'C', 'B', 'E', 'Z'} mapping)
        # in which the child is also the parent
        for current_parent, current_children in mapping.items():
            if parent in current_children and child != current_parent:
                current_children.add(child)

    return mapping

for k, v in relationship_dictionary(df, "Parent", "Child").items():
    print(k, v)

Result:

A {'E', 'B', 'Z', 'C'}
B {'E', 'A', 'Z', 'C'}
C {'A', 'B', 'Z', 'E'}
D {'Z', 'E'}
E {'Z'}
F {'Z'}

I have only tested it with your example but you might want to verify the code works with some more samples!

Hope it helps!


Sliding marker on link hover with jQuery

JavaScript

This code violates the Don't Repeat Yourself principle because there is some redundancy in the code to handle the hover events, but for a beginner it might be challenging to know how to abstract the common code.

One way to reduce the redundancy is to define a mapping of id values to classes to toggle. This can be done with a plain-old JavaScript Object, like below, or with a Map() object.

const targetClassMapping = {
  'menulink2': 'movedown1',
  'menulink3': 'movedown2'
};

Then the code can utilize .hover() on any element with the class name menulink with a single event handler that checks the id attribute (using the jQuery .attr() method) of the event target and if the id matches a key in that mapping it then calls .toggleClass() on the slider element. Additionally, $('slider') can be stored in a variable and used when needed instead of querying the DOM each time.

const slide = $('#slide');
$('.menulink').hover(function(){
  const id = $(this).attr('id');
  if (id && id in targetClassMapping) {
    slide.toggleClass(targetClassMapping[id]);
  }
});

This could also be done without jQuery, though the code wouldn't be quite as succinct.

const slider = document.getElementById('slide');
const targetClassMapping = {
  'menulink2': 'movedown1',
  'menulink3': 'movedown2'
};
['mouseover', 'mouseout'].forEach(type => document.addEventListener(type, handleMouseEventOnMenuLink));
function handleMouseEventOnMenuLink(event) {
  if (event.target.parentNode) {
    const id = event.target.parentNode.id;
    if (id in targetClassMapping) {
      const method = event.type === 'mouseover' ? 'add' : 'remove';
      slider.classList[method](targetClassMapping[id]);
    }
  }
}

CSS / SASS

I see a couple lines with !important.

.movedown1
  margin-top: 40px
  background-color: blue !important

.movedown2
  margin-top: 80px
  background-color: green !important

It is wise to not depend on !important if possible, to avoid confusion. You can remove the !important from those background color styles if you make the selectors more specific than just #slide - e.g. #slide.movedown1 and #slide.movedown2.


C program to create and print a linked list from the command line

Nice formatting, good variable and function names. I learned more about getopt() from the program (I used to have to write my own command line parsers)! Good use of system macros such as EXIT_FAILURE.

Using Typedef

If the code used typedef for the definition of node, the code might be slightly shorter and more readable:

typedef struct node{
    int value;
    struct node *next_ptr;
} Node;

Node *head;

void add_node(int data){
    Node* temp = head;

    /* ... */
}

void print_nodes(void){
    Node* temp = head;

    /* ... */
}

By using typedef a new type is created. This also might decrease the possibility of future errors by forgetting to put the struct in at some point. This stackoverflow question discusses why it might be good to use typedef.

Global Variables

Generally the use of global variables are frowned upon. When creating, reading and debugging code global variables can be affected by side affects and it can be very difficult to find where the problem is actually occurring. This stackoverflow question talks about when it is proper to use global variables.

It might be better if the global variable NODE *head was declared in main() and then passed by reference into each function that modified it, and passed by value into each function that only used it and didn't change it.

Passing head into each of the functions would make the following changed necessary:

        case 'p':
    /*+>*/  print_nodes(head);      /* Pass by value */
            break;

            while(token != NULL){
                node_value = strtol(token, &ptr, 10); /*Convert each string to integer*/
                if((*ptr) != 10 && (*ptr) != 0){ /*If it's not a newline or a null then invalid input*/
                    fprintf(stderr, "Invalid number: %c", *ptr);
                    exit(EXIT_FAILURE);
                }
    /* ++> */   add_node(node_value, &head);        /* Pass by reference */
                token = strtok(NULL, s);
            }

void add_node(int data, Node **head){
    Node* temp;

    if(*head == NULL){
        *head = malloc(sizeof(struct node));
        (*head)->value = data;
        (*head)->next_ptr = NULL;
        return;
    }

    temp = *head;
    while(temp->next_ptr != NULL){
        temp = temp->next_ptr;
    }

    if((temp->next_ptr = malloc(sizeof(struct node))) == NULL){
        fprintf(stderr, "Out of memory");
        exit(EXIT_FAILURE);
    }

    temp = temp->next_ptr;
    temp->value = data;
    temp->next_ptr = NULL;
}

void print_nodes(Node* head){
    Node* temp = head;
    if(temp == NULL){
        printf("Linked list is empty\n");
    }
    for(temp = head; temp != NULL; temp = temp->next_ptr){
        printf("%i\n", temp->value);
    }
}

This example might make the program safer and easier to debug and read.

Implicit Type Conversion

My compiler flagged the following line as an implicit type conversion:

            add_node(node_value);

because node_value is declared as a long rather than int,

    long node_value;

void add_node(int data) { /* ... */

If node_value needs to be a long because that's what strtok() is returning, it might be better to either change the input type for add_node() or to explicitly cast node_value in the call:

            add_node((int) node_value);

The actual warning message I get is implicit conversion loses integer precision: 'long to int' (Xcode 8.2 on El Capitan).

Functions that might be helpful

To implement a full linked list program some functions that might be helpful are:

NodePointer new_node(int value);
NodePointer delete_node_by_value(int value, NodePointer head);
NodePointer delete_node_by_pointer(NodePointer delete_target, NodePointer head);
NodePointer find_node(int value, NodePointer head);
void print_node(int value, NodePointer head);       // called by print_nodes

Use of the exit() Function

The use of the exit() function can be problematic, in a large software system that one is writing only a piece of, it would be better to return an error code from the add_node() function rather than call exit. In some cases such as operating systems calling exit() can have dire
consequences (shut down).

The C programming language was originally created to implement operating
systems and in some cases is still used for that purpose. While C doesn't have the exception throwing capabilities of C++, Java, C# and other more modern languages errors can be handled, either by returning error codes or using setjmp() and longjmp().


The question could benefit from some context. You don't need to answer every one of the following, but it would be helpful for reviewers.

Why have you written a morse code converter?
Is this going to be used by anyone else or is it a personal project? Is it a homework question? From this we can tell if the review should focus on finding security holes, or just annoying bugs.
What will the input look like? Can we assume nobody is going to call convert_to_word with a letters that aren't dot, dash, or space?

def convert_to_morse_code(word):
    ...

def convert_to_word(word):
    ...

This looks a little funny, as they both take a "word", yet neither does. One takes a string which may have complete sentences, and the other takes a string containing morse code. I'd suggest adding a docstring for these functions which gives a brief explanation about the expected input.

if letternum.isalnum():
    string += f'{morse_code[letternum.upper()]} '
else:
    string += '  '

What happens if somebody comes along and helpfully adds some international morse code symbols such as "?" or "+" to the dictionary? This code will ignore those. I think the dictionary should be the source of truth, not isalnum.

if letternum.upper() in morse_code:
    string += morse_code[letternum.upper()] + ' '
else:
    string += '  '

The above possibility shows that letternum is no longer an appropriate name. Lets update it the name to c or char, short for character, which is what it is. The other change I would propose here is to use the dictionary method .get as it has a handy parameter 'default value'. If we set the default value as '  ' (for when the character is not a symbol we know how to write in morse code) this takes the place of the else branch.

for char in word:
    string += morse_code.get(char, ' ') + ' '

I think it would be worth noting in the docstring for this function that unrecognised characters are skipped, and this is the line responsible.

for code in word:
    if code == '/':
        word = word.replace(code, '   ')

In general it is a bad idea to modify the thing you are looping over. It can lead to hard to spot bugs. The other thing to note is that the first time '/' is encountered, all occurrences of it are replaced. This doesn't need to be in a loop.

    for key in morse_code.keys():
        if morse_code[key] == code:
            string += key

This loop does a little more work than is necessary. It will get slower and slower each time a new symbol is added to the dictionary. The performance loss will honestly not be noticeable, but if you think the solution sounds nicer you can try implementing it.

The solution is to build a new map which is the inverse of morse_code, where all the keys become values and all the values become keys.

morse_code = {...}
code_to_char = {code: char for char, code in morse_code.items()}
...

    if code in code_to_char:
        string += code_to_char[code]

There are some small problems to which you will need to decide on the answer.

convert_to_word(convert_to_morse_code("Hello, world!")) outputs 'HELLO WORLD '. Is that ok? Should the output match the input more closely? Why is there trailing space?
convert_to_morse_code("HI LOW") outputs '.... ..   .-.. --- .-- '. Some variations exist, such as using a slash to indicate a space ('.... .. / .-.. --- .--') or seven dots (.... .. ....... .-.. --- .--). Could your code have that as an optional feature?
convert_to_word works on some weird input, and not on others. But it does not ever tell the user when something is wrong. I'd prefer if convert_to_word('--- -, .') failed in some way rather than returning 'OE'.


Your code is readable, but it doesn't feel very "rubyish" to me.   I think that's due to needing to setup and track some extra variables and track the index of the word in question -- having to case it depending on the index, etc.

It also bugs me a little bit that title= is doing so much.  This is how I would have written it:

class Book
  def title
    @title
  end

  def title=(title)
    @title = titlieze(title)
  end

  private
  def titlieze(title)
    stop_words = %w(and in the of a an)
    title.capitalize.split.map{|w| stop_words.include?(w) ? w : w.capitalize}.join(' ')
  end

end

I used stop_words instead of special_words since that's a common naming scheme for search applications (ie. words you ignore when searching).  I'd be tempted to move that into a constant or some other configuration, but it works fine for this.

Rspec output:

$ rspec -f d foo_spec.rb

Book
  title
    should capitalize the first letter
    should capitalize every word
    should capitalize every word except...
      conjunctions
      prepositions
      articles
        the
        a
        an
    should always capitalize...
      I
      the first word

Finished in 0.00261 seconds
9 examples, 0 failures


Firstly your formatting looks god awful - I hope that this is just from pasting it here.

Then you should rename your session variable. It isn't ok it's the time of last request - so $_SESSION['lastRequest'] seems like a good name.
Also that 4 seconds shouldn't be just some random number but a named constant that is used in all places and then can be changed easily.

Now about the effectiveness:
I don't know what the latest advancements in the bot-scene are. However I'm quite certain that XRumer still is one of the better ones, so I'll try to match your system against that.

Your strongest protection is the captcha. There always are claims that those are bypassed - you can half the boasted recognition rate and probably still be on the safe side.

Your email-confirm trick is easily detectable - just check for the display:none and not fill it out.
Whether that is actually done depends on some factors:

Whether the bot is even able to do this. But detecting the trick in the current form isn't hard - I'm quite certain XRumer is able to detect that.
Whether the bot WANTS to do that. Spamming is only about volume. Parsing the site to detect such cheap tricks takes time, memory and CPU load.
Most probably the spammers do not care about your site in particular; They might set their spambot to the fastest mode and blast away. Maybe at different times the bot is configured to try the best it can to post its message - and your current trick fools no one.

However you can just create another CSS id for the hidden fields and use that.
As said, spamming is mostly done as quick as possible. Downloading and parsing further files is avoided - and I'm certain it will stay that way.

So in conclusion: It may not stop everything, but it will stop a lot.


You are trying to count each key separately.  This means you need to iterate through the entire list to count each key.  Instead you can keep a running total of your key's and only have to iterate through your list once:

string[] source = { "test1", "test2", "test3", "test4", "test1", "test1", "test3" };
Dictionary<string, int> dic = new Dictionary<string, int>();

foreach(string s in source){
    if(dic.Keys.Contains(s))
         dic[s] = dic[s]++;
    else
       dic.Add(s, 1);
}

EDIT: I did not include getting the max value as what you have works for that and has already been re-written by thantos


Some comments on the style of your code.

Formatting. Your code is hard to read because there are no spaces.
Also the formatting seems to be off. It might be easier to read like this:

int main(void)
{
    int T;//number of test cases
    scanf("%d", &T);
    while (T--) {

        char str[20];
        scanf(" %[^\n]", str);

        int i, j, k, m, count = 0, l, p = 1, n;
        l = strlen(str);

        for (i = 0; i < l; i++) {
            if (str[i] == ' ') {
                count++;
            }
        }

        m = count + 1;
        char arr[m][50];
        int check[m], func[m];

        for (i = 0, k = 0, j = 0; j <= l; j++) {
            if (str[j] == ' ' || j == l) {
                check[i] = 0; func[i] = 1;
                arr[i][k] = '\0';
                i++;
                k = 0;
            }
            else {
                arr[i][k] = str[j];
                k++;
            }
        }

        for (i = 0; i < m; i++) {
            for (j = i + 1; j < m; j++) {
                if (check[i] == 0 && strcmp(arr[i], arr[j]) == 0) {
                    check[j]++;
                    func[i]++;
                }
            }
        }

        for (i = 0; i < m; i++) {
            p *= factorial(func[i]);
        }

        n = factorial(m) / p;
        printf("%d/%d\n", 1, n);
    }
    return 0;
}

Don't use comments for what could be described in the code directly. This:

    int T;//number of test cases
    scanf("%d", &T);
    while (T--) {

Should be really this:

    int number_of_test_cases;
    scanf("%d", &number_of_test_cases);
    while (number_of_test_cases--) {

Avoid long functions for maintainability and readability. You wrote one function for the factorial calculation; good. But now take a look
    at the main() function. Can you guess what the steps are in the while
    by looking at it? There's too much stuff going on in the main().
    Isolate parts which do one thing into a function. For example this
    piece:

    int i, j, k, m, count = 0, l, p = 1, n;
    l = strlen(str);

    for (i = 0; i < l; i++) {
        if (str[i] == ' ') {
            count++;
        }
    }

First of all I would write the variables on several lines to have it more readable:

    int i;
    int j;
    int k;
    int m;
    int count = 0;
    int l;
    int p = 1;
    int n;
    l = strlen(str);

    for (i = 0; i < l; i++) {
        if (str[i] == ' ') {
            count++;
        }
    }

Then we realize that j, k, m, p and n are not even needed yet at this point, so we put
 them after all this:

    int i;
    int count = 0;
    int l;
    l = strlen(str);

    for (i = 0; i < l; i++) {
        if (str[i] == ' ') {
            count++;
        }
    }

    int p = 1;
    int j;
    int k;
    int m;
    int n;

Then another thing is l can be directly initialized with strlen(str). Also I renamed
it everywhere in the program to length which  states more easily for a reader what it
does.

This gives us this:

    int i;
    int count = 0;
    int length = strlen(str);

    for (i = 0; i < length; i++) {
        if (str[i] == ' ') {
            count++;
        }
    }

Now we should make it a function so it looks like this:

    int length = strlen(str);
    int count = get_count_of_whitespace(&str, length);

Proceed with the rest of the main function like that by refactoring it into
manageable chunks, and probably with that alone you will find some mistakes or improvements
for your algorithm.


Increasing performance of OpenMP based advection equation solver for Xeon Phi

You should first check for the number of command line arguments via argc.  If there are too few, print an error message and then terminate the program.

if (argc < 3) {
    // print an error message...
    return EXIT_FAILURE;
}

In addition, your first existing check in main() should also return EXIT_FAILURE, instead of calling exit(1).  The latter is non-portable, unlike the former, and it's already safe to return from main() if the program cannot continue execution.  More info about that here.
All of your erroneous outputs shouldn't be printed to printf():

printf("ERROR: Number of cells should be integral multiple of number of threads \n");

They should instead be printed to fprintf() and stderr:

fprintf(stderr, "Number of cells should be integral multiple of number of threads \n");

This comment is misleading:

// initialize
u[0:nx] = 0.0;
u[nx/4:nx/2] = 1.0;
f[0:nx+1] = 0.0;
res[0:nx] = 0.0;

They're not actually being initialized; they're being assigned after declaration.  Initialization involves giving a variable its type and its initial value.  You should do that here, which will keep variables as close in scope as possible, which is generally preferred for maintenance concerns.

Either way, you don't really need that comment.  It's already clear what's being done (even after making this correction), and comments shouldn't state the obvious.
Also regarding closest possible variable scope: you have i declared towards the top of main(), but you don't use it until the for loop towards the end.  If you don't have C99 (and thus cannot initialize i within the loop statement instead), declare i right before the loop.
This line doesn't look right:

td.maxthreads = omp_get_num_threads();

It appears that you're looking for omp_get_max_threads():

td.maxthreads = omp_get_max_threads();

This will return the maximum number of available threads for a parallel environment.  If this is not really your intent, then rename maxthreads to something more accurate.


Django models: different methods on each instance?

What you want is inheritance as a proxy model.

In your subclasses, use the following Meta option:

class SlimyMonster(Monster):
    class Meta:
        proxy = True

    def some_method_to_override(self):
        ...

This way the subclassing has no effect on the database side, only on the Python side.


Count number of matching nodes in a Binary tree

I'll leave the comments on build_tree() as they stand in my other answer, and focus on your match_count() method:

Bad parameters for method? – It feels a little constructed, and special, to test for two key values. Why not one, or three, or an arbitrary number? Or a range?
Not optimal method name? – To me, it sounds a little strange with match_count. I just think of safety matches... I think I would prefer just count() (or count_matching()). Do note that when using count() we kind of hide/reimplement the default count() of Python. If your binary tree was within a class (as in my test cases) that could work nicely, but outside of classes it should probable be avoided and you should rather use the count_matching or match_count name.
Consider use of local variables – I don't oppose to using local variables like count_left and count, but in this rather simple case you could actually do:

def count(node, key1, key2):
    """Counts the number of node that matches key1 or key2 in the subtree."""
    if node is None:
        return 0

    return ( (node.key == key1 or node.key == key2)
             + match_count(node.left, key1, key2)
             + match_count(node.right, key1, key2) )

Use predicates to generalize method

If you look at the accepted answer of "Most pythonic way of counting matching elements in something iterable", they suggest some neat ways of handling your issue at hand. In fact your match_count() can be replaced with the itertools.quantify recipe.

def quantify(seq, pred=None):
    "Count how many times the predicate is true in the sequence"
    return sum(imap(pred, seq))

You could either use this directly, and reuse any of the order traversals, like in:

def count(node, pred=None):
    """Count how many times the predicate is true in binary tree."""
    return sum(imap(pred, inorder(node)))

Another version would be to use the predicate similar to the first count() version above:

def count(node, pred=None):
    """Count how many times the predicate is true in binary tree."""
    if node is None:
        return 0

    return (not pred or pred(node.key)) + count(node.left, pred) + count(node.right, pred)

Have added a not pred in front of the test to allow for using this method with default parameter of None in which case it ends up counting all elements in binary tree.

This general approach opens up for counting whatever you feel like, but you need to call it slightly different. Here are some examples:

# Two variants to test against two different key values
# Note that the lambda function will receive the node.key value
count(root, lambda x: x == 'D' or x=='E')
count(root, lambda x: x in 'DE')

# or use a designated method (accepting
def is_vowel(char):
    return char in 'AEIOU'
count(root, is_vowel)

Complexity considerations

Time complexity of any of these implementations (including your original one) will be \$O(n)\$. Slightly different memory complexity, but neither algorithm will use much extra memory (with the exception when using a list implementation of inorder() which duplicates the binary tree into a list thusly requiring more memory).


General points

both wSheet and destSheet points to "Sheet2", I suppose you don't need both
while you can save a few lines using :, it's generally considered to be a bad practice to use them.
functions are generally more or less portable, hardcoding sheet names in them makes it impossible. You could pass sheet names as arguments
lookUpRange is not declared

You should always use Option Explicit in your code

Dim Found As Range - it's name suggests it's a boolean, not a range
lr = wSheet.Cells(wSheet.Rows.Count, "A").End(xlUp).Row - you set the value at the beginning, but never use it, just overwrite
ActiveWorkbook.Sheets("Sheet2").Range("P1").Value = "LookupVal" You've declared variables for Sheet2, why don't use them?
... and a lot of other similar issues

General performance

you can use these at the beginning of your codeto speed up your code:
Application.ScreenUpdating = False
Application.Calculation = xlCalculationManual
to perform calculations after inserting the formulas use
Application.calculate

Specific suggestion

.Range("$A:$C").AutoFilter Field:=2, Criteria1:="<>"
Set Found = .Range("A:A").Find(What:=wSheet.Cells(i, "A").Value, _
    LookIn:=xlValues, LookAt:=xlWhole, SearchOrder:=xlByRows, SearchDirection:=xlNext, _
    MatchCase:=False)
If Not Found Is Nothing Then
    wSheet.Cells(i, "P").Value = Found.Offset(, 2).Value
End If

It's difficult to give the best solution without looking your data, but here I'd definitely use some formula (Index / match / row / ...) instead of filtering the data thousands of times. Or at least sort data to avoid some of the filtering.


Abstract a common function out:

var myFunction = function(myClass){
  $("#prevTex").removeClass (function (index, css) {
    return (css.match (/\btextc-\S+/g) || []).join(' ');});
  $("#prevTex").addClass(myClass);
});

Then apply it like so:

$(".black-sel").click(function(){ myFunction("textc-black"); });
$(".white-sel").click(function(){ myFunction("textc-white"); });
$(".red-sel").click(function(){ myFunction("textc-red"); });


public ActionResult ViewInvoice(Guid invnum)

Regardless that one could figure out that invnum means "invoice number" this name isn't good. First it isn't a number but a globally unique identifier and second using abbreviations for variable naming shouldn't be done because it reduces readability.

-> how about renaiming this to invoiceId ?

int loggerInCustomerId = GetTheLoggedInCustomerId();

The variable name is misspelled you should change it to loggedInCustomerId

The "validation" part wether a invoice belongs to the current loggedin customer should be done by querying the database (if one is used) to get the invoice which matches the invoiceId and the loggedInCustomerId. An overloaded GetInvoice(GUID, int) could be used in the business layer (will be _invoiceLogic) which shouldn't throw an exception but return either null or some Invoice.Empty constant. Throwing an exception doesn't buy you much but will slow down the application because you will also need to handle it.

Maybe a method like the Dictioary<T>.TryGetValue() would be a good fit so one could write something along this lines

public ActionResult ViewInvoice(Guid invoiceId)
{
    int loggedInCustomerId = GetTheLoggedInCustomerId();
    Invoice invoice;

    if (!_invoiceLogic.TryGetInvoice(invoiceId, loggedInCustomerId, out invoice)
    {
        return RedirectToAction("Index", "MyInvoices");
    }
    //do other stuff as normal
}

A version without accessing the data layer could then look like

public bool TryGetInvoice(Guid invoiceId, int customerId, out Invoice invoice)
{
    invoice = GetInvoice(invoiceId);
    if (invoice.CustomerId == customerId)
    {
        return true;
    }

    invoice = null;
    return false;
}

A version with a fictive data layer (because I don't know if you use one) could look like this

public bool TryGetInvoice(Guid invoiceId, int customerId, out Invoice invoice)
{
    invoice = _dataLayer.GetInvoice(invoiceId, customerId);

    return invoice != null;
}

If you use a TryGetXXX method you need to make sure that such a method never throws an exception because it is expected that it doesn't.


Dependency injection with reading a file

You have implemented dependency injection correctly. IReadFileTypes is injected into Program. In terms of the correctness of your example however, pretty much everything else is completely wrong! Don't worry though, we all have to start somewhere, so I will offer some tips. These tips might help you with this example, but if you want to improve, you should probably do some personal training/studying. Pluralsight is a really nice place to start. They have some beginner C# courses that you can do with a free trial. Anyway, on with the code review!

Streams implement IDisposable, and should always be disposed of when used. Typically this is done with using block.

Builder is not a builder, it is a factory. It's job is to create instances of IReadFileTypes. This shouldn't be done in it's constructor because then you need a new factory, each time you want a new IReadFileTypes.

async void should almost always be avoided. Try to return Tasks from async methods so they can be awaited by the caller. A Task should be returned from ReadUsingFileSteamAsync() and have it's Wait() method called.

The point of an interface is to abstract away the implementation details from the client. IReadFileTypes has the method ReadUsingFileSteamAsync() which publicises exactly what the implementation is. This is called a leaky abstraction. When writing an interface, think about what other implementations might look like. One thing that you do know about all implementations is that they are going to need a file path. An alternative interface might then look something like this

public interface IFilePrinter
{
    Task PrintFileAsync(string filePath);
}

You could imagine this interface being implemented by e.g. ColoredFilePrinter, VerboseFilePrinter e.t.c.

ReadfileFromSystem is a bit nuts! You have wrapped _reader.ReadToEnd() in a Task.Run() and awaited that. Instead you could just await _reader.ReadToEndAsync().

Putting all of this together, your example could look something like

public class Default
{
    static void Main()
    {
        var printerFactory = new FilePrinterFactory();
        var filePrinter = printerFactory.CreateDefaultPrinter();

        var program = new Program(filePrinter);
        program.DoStuff();

        Console.ReadLine();
    }
}

public interface IFilePrinter
{
    Task PintFileAsync(string filePath);
}

internal class DefaultFilePrinter : IFilePrinter
{
    public async Task PintFileAsync(string filePath)
    {
        using (var reader = File.OpenText(filePath))
        {
            var fileContent = await reader.ReadToEndAsync();
            Console.WriteLine("File content: " + fileContent);
        }
    }
}

public class FilePrinterFactory
{
    public IFilePrinter CreateDefaultPrinter()
    {
        return new DefaultFilePrinter();
    }

    public IFilePrinter CreateSpellCheckingPrinter(string language)
    {
        throw new NotImplementedException();
    }
}

public class Program
{
    private const string FilePath = @"C:\Brackets\Projects\index2.html";
    private readonly IFilePrinter _filePrinter;

    public Program(IFilePrinter filePrinter)
    {
        // I don't care what kind of printer I have, I just know that I need to print a file!
        _filePrinter = filePrinter;
    }

    public void DoStuff()
    {
        _filePrinter.PintFileAsync(FilePath).Wait();

        // TODO: Do more stuff
    }
}

PS: In case you are interested, you can do the whole thing in 1 line of code

Console.WriteLine("File content: " + File.ReadAllText(@"C:\Brackets\Projects\index2.html"));


The main issue I see is that the looping doesn't accomplish anything useful, since you never reference the li that you iterate over - just take the sortOrder from the form and concatenate it into the selectors you try to find.

Because you reference the #References_##_UI twice, consider saving it in a variable first - or, even better, since this is jQuery, you can chain methods on the selected collection.

It sounds like the data.referenceText is text, not HTML markup - in which case you should insert it into the DOM with .text, not with .html. (.text is faster and safer)

$('#submit-form').submit(function (event) {
  event.preventDefault();
  const sortOrder = $(this).data('sortorder');
  $(`#References_${sortOrder}_UI`)
    .text(data.referenceText)
    .prop('href', data.GuidelineExternalReference);
  $(`input[name='References[${sortOrder}].Text']`).val(data.referenceText);
  $(`input[name='References[${sortOrder}].Link']`).val(data.GuidelineExternalReference);
  $(`input[name='References[${sortOrder}].GuidelineId']`).val(data.Value);
});

The above looks mostly reasonable to me, but I'd change the HTML too, if that's permitted. Numeric-indexed IDs are never a good idea; IDs should be reserved for singular, unique elements. (You can also consider not using IDs at all, because every time there is one, a global variable is created, and globals can result in confusing behavior).

A related issue is that the submit handler here is attached to:

$('#submit-form').submit(

Since IDs must be unique in a document, this will only attach a listener to a single form, but it sounds like you have multiple forms that you want to listen for submit events to.

To solve the duplicate IDs and the numeric-indexed IDs, use the already-existing class to select the forms instead, and once you have a reference to the form in the handler, use .find to select its children elements that need to be populated.

Your 3 hidden inputs are somewhat repetitive. It might look somewhat tolerable now, but if you might add more, or for the general case of linking each data property name to a particular input, consider using an object or array linking each property to the selector:

const inputNamesByDataProps = {
  referenceText: 'Text',
  GuidelineExternalReference: 'Link',
  Value: 'GuidelineId',
};
$('.form-horizontal').on('submit', (event) => {
  event.preventDefault();
  const $this = $(this);
  $this.find('a')
    .text(data.referenceText)
    .prop('href', data.GuidelineExternalReference);
  for (const [dataProp, inputName] of Object.entries(inputNamesByDataProps)) {
    $this.find(`input[name$=${inputName}]`).val(data[dataProp]);
  }
});

(The [name$=${inputName}] means: "Find an element whose name attribute ends with what's in inputName")

<form class="form-horizontal" data-sortorder="1">

Update Values

  Testing html.
  <input type='hidden' name='References[1].Index' value="1">
  <input type='hidden' name='References[1].Link' value="www.oldlink.com">
  <input type='hidden' name='References[1].Id' value="88">

    <button type="submit">Save</button>
    <button type="button" data-dismiss="modal">Close</button>

</form>

The input HTML name attributes look pretty repetitive too, but repetitive HTML usually isn't something to worry about, especially if your backend logic is easier to work with when the attributes are like References[1].Index. But if you wanted, you could change it to something like

<input type='hidden' name='sortorder' value="1">
<input type='hidden' name='Index' value="1">
<input type='hidden' name='Link' value="www.oldlink.com">
<input type='hidden' name='Id' value="88">

putting the [1] into the hidden sortorder input instead, and then parse the form values on the backend.


Haikuifier (Or at least Haiku Identifier)

You should be more helpful to the user, both when the entered sentence can't be made into a haiku (too short or too long) and when not finding the word in the dictionary. At least tell which word was not found in the dictionary.
Also when thefreedictionary.com is not reachable (for whatever reasons), your code will insult the user by telling him he can't spell.
You dictionary lookup fails for some words:

"sparrow's", or any possessive form (raises SyntaxError)
"banging", or any gerund form. (gives the syllables of the infinitive instead)
Any punctuation (also raises SyntaxError)

I tested your program with this haiku from wikipedia:

Snow in my shoe
Abandoned
Sparrow's nest
— Jack Kerouac, collected in Book of Haikus, 2003

And this:

Whitecaps on the bay:
A broken signboard banging
In the April wind.
— Richard Wright, collected in Haiku: This Other World, 1998

The first raises a SyntaxError and the second puts In on the end of the second line because of the gerund counting as one less syllable.
What your code also does not catch is that thefreedictionary puts a flood mechanism into place after some time. I found this out by testing my own script. In this case the fifth request in a row returns with HTTP code 403 (Access forbidden).

How I would write it:
import requests
import sys
import re

def get_num_syllables(value):
    if value.endswith("ing"):
        return get_num_syllables(value[:-3]) + 1
    try:
        res = requests.get("http://www.thefreedictionary.com/" + value)
    except requests.exceptions.ConnectionError as e:
        print("Can't connect to dictionary website: http://www.thefreedictionary.com/" + value)
        sys.exit(1)
    if res.status_code != 200:
        raise requests.exceptions.ConnectionError("Can't connect to dictionary website: http://www.thefreedictionary.com/{}, HTTP code {}".format(value, res.status_code))
    value_re = re.compile(value)
    for word in res.text.split():
        if "<h2>" not in word:
            continue
        word = word[word.find("<h2>") + 4:word.find("</h2>")].lower()
        if '·' in word:
            match = re.match(word.replace('·', ''), value)
            if not match:
                continue
            return word.count('·') + 1
    not_founds = ("Word not found in the Dictionary and Encyclopedia.",
                 "is not available in the general English dictionary")
    if any(not_found in res.text for not_found in not_founds):
        raise SyntaxError("Can't find word in dictionary: {}".format(value))
    return 1

def get_num_syllables_all(words):
    syllables = {}
    for word in words:
        word = word.lower()
        if word in syllables:
            continue
        syllables[word] = get_num_syllables(word)
    return syllables

def haiku(words, haiku_form):
    # words = "Whitecaps on the bay A broken signboard banging In the April wind".split()
    num_syllables = get_num_syllables_all(words)

    lines = [[] for _ in range(len(haiku_form))]
    count, line = 0, 0
    for word in words:
        count += num_syllables[word.lower()]
        if count > haiku_form[line]:
            raise SyntaxError("Not possible to split word at syllable boundary: {}, {}".format(word, syllables))
        lines[line].append(word)
        if count == haiku_form[line]:
            line += 1
            count = 0

    # validate the haiku
    for i, line in enumerate(haiku_form):
        sum_line = sum(num_syllables[word] for word in lines[i])
        if sum_line > line:
            raise SyntaxError("Too many syllables in line {}. Got {}, want {}".format(i, sum_line, line))
        elif sum_line < line:
            raise SyntaxError("Not enough syllables in line {}. Got {}, want {}".format(i, sum_line, line))
    return "\n".join(" ".join(line) for line in lines)

if __name__ == '__main__':
    haiku_form = (5, 7, 5)
    words = input("Please enter the words to use: ").strip().split()
    try:
        print(haiku(words, haiku_form))
    except SyntaxError as e:
        print(e)