Get missing sequential numbers from array I have an array like Original = {1, 2, 3, 6, 7, 8, 9, 10, 12}. After removing some elements like Delete = {6, 7} from it, I want to get n sequential numbers which are not in the array, like IDs = {4, 5, 6, 7, 11, 13, 14, ..., n}.

After getting the numbers I want the original array to be changed to the updated array.

public static ushort GetNextID(ref List<ushort> Updating)
{
    for (var i = 0; i < Updating.Count; i++)
    {
        if (Updating[i] != i + 1)
        {
            Updating.Insert(i, Convert.ToUInt16(i + 1));
            return Convert.ToUInt16(i + 1);
        }
    }
    Updating.Add(Convert.ToUInt16(Updating.Last() + 1));
    return Convert.ToUInt16(Updating.Last() + 1);
}
public static ushort[] GetNextIDs(ref List<ushort> Updating, ushort Count)
{
    List<ushort> IDs = new List<ushort>();
    for (var t = 0; t < Count; t++) IDs.Add(GetNextID(ref Updating));
    return IDs.ToArray();
}
static void Main(string[] args)
{
    IEnumerable<ushort> Original = new ushort[] { 1, 2, 3, 4, 5, 6, 7, 9, 10 };
    IEnumerable<ushort> Delete = new ushort[] { 4, 5, 6 };
    var Updating = Original.Except(Delete).ToList();

    // Expected: 4, 5, 6, 8, 11, 12, 13, 14
    // Outputs:  4, 5, 6, 8, 12, 13, 14, 15
    foreach (var id in GetNextIDs(ref Updating, 8)) Console.WriteLine(id);

    Console.WriteLine();

    // 1, 2, 3, (4, 5, 6), 7, (8), 9, 10, (11, 12, 13, 14)
    foreach (var i in Updating) Console.WriteLine(i);

    Original = Updating;
    Console.ReadKey();
}

I'm using refs for performance but I'm not sure if it does any good.

Naming

In C#, it is conventional to use camelCase (first letter smallcase) for method parameters and local method variables. So original instead of Original, and count instead of Count.

I also find Updating a quite confusing name. Usually variable names are nouns, whereas this sounds like a verb or adjective. Plus in Main, by the time it's been assigned a value it's not going to change any more anyway

ref

The ref keyword is irrelevant to what you're doing. I'm not sure what performance benefit you expect them to have, but I'd guess it's based on a misunderstanding of what the keyword actually does. It makes the code a little harder to read too, because it's hard to understand why the keywords are there.

Single-line for loop

In your code you have:

for (var t = 0; t < Count; t++) IDs.Add(GetNextID(ref Updating));

There are three other reasonable ways you could write this:

for (var t = 0; t < Count; t++) { IDs.Add(GetNextID(ref Updating)); }

for (var t = 0; t < Count; t++)
    IDs.Add(GetNextID(ref Updating));

for (var t = 0; t < Count; t++)
{
    IDs.Add(GetNextID(ref Updating));
}

Of these four, the one you picked is my least favourite. It's not found very commonly in C# so there's some cognitive friction when reading something written in that style.

Of the other three that I posted, most people consider the second to be bad practice because it is easy to make a mistake when inserting additional lines. I don't wholly buy into this personally but you should still probably pick one of the other two.

Types

While technically your numbers fit into ushorts, I wouldn't get into a habit of using integer types other than int unless you have a reason to. Unfortunately C# has relatively poor support for using multiple numeric types together in certain situations- e.g. as a generic type parameters.

Having to put Converts all over the place takes time, makes the code harder to read, and is dangerous as you end up with potential run-time (rather than compile-time) failures.

This isn't to say don't use other integer types, just make sure there's a good reason, and think through whether there's likely to be issues in the future. I've been bitten by this a few times myself.

Side-effects

It's unexpected that GetNextID would modify Updating, especially as the method returns something. You should probably have it just return the value, then let the method which calls it take charge of inserting the value. At the very least, give it a name that makes it clear it will have a side-effect beyond just returning a value.

LINQ extension power

Jude M already mentioned Enumerable.Range. There are other very powerful LINQ extensions that reduce this entire task to a few very terse lines of code.

var exclude = original.Except(delete);
var unused = Enumerable.Range(0, Int32.MaxValue).Except(exclude).Take(8);

Enumerable.Range is lazy, so that scary Int32.MaxValue won't actually have a performance impact, this will only keep evaluating until it finds 8 values not in your list.


Find most occurring word in a txt file Assume that we have a .txt file that has one word per line.
Find out the word that occurs the most.

Here's what I was able to write (I used array of strings instead of a file in this example):

string[] source = { "test1", "test2", "test3", "test4", "test1", "test1", "test3" };
Dictionary<string, int> dic = source.Distinct().ToDictionary(p => p, p => 0);
var keys = new List<string>(dic.Keys);
foreach (string key in keys)
{
  dic[key]=source.Count(f => f == key);
}
int max = dic.Values.Max();
foreach (KeyValuePair<string, int> kvp in dic)
{
    if (kvp.Value == max)
    {
        Console.WriteLine(kvp.Key + " " + max);
        break;
    }
}

Questions:

Can this be done better and more efficient way (speed/ memory)?
What if file size is 10GB. How would you do it differently from my approach?

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


Quiz component with different but similar data models I have a component with this template:

<ion-card *ngIf="isActive">
  <ion-card-header>{{question.question}}</ion-card-header>

  <ion-grid>
    <ion-row>
      <ion-col *ngFor="let answer of question.answers">
        <button ion-button outline (click)="answer.action()">{{answer.text}}</button>
      </ion-col>
    </ion-row>
  </ion-grid>

</ion-card>

And the code:

@Component({
  selector: 'extra-card',
  templateUrl: 'extra-card.html'
})
export class ExtraCard {
  public static readonly EXTRA_CARD_TYPE_1: string = "type_1";
  public static readonly EXTRA_CARD_TYPE_2: string = "type_2";

  @Input() type: string = ExtraCard.EXTRA_CARD_TYPE_1;

  isActive: boolean = true;
  question;

  constructor(
    private ga: GoogleAnalytics,
    private socialSharing: SocialSharing,
    private appRateService: AppRateService
  ) {}

  ngOnInit() {
    switch (this.type) {
      case ExtraCard.EXTRA_CARD_TYPE_1:
        this.makeExtraCardType1();
        break;

      case ExtraCard.EXTRA_CARD_TYPE_2:
        this.makeExtraCardType2();
        break;
    }
  }

  private makeExtraCardType1() {
    this.question = {
      question: "Some question",
      answers: [
        {text: "Answer 1", action: () => {this.action1Type1();}},
        {text: "Answer 2", action: () => {this.action2Type1();}},
        {text: "Answer 3", action: () => {this.action3Type1();}}
      ]
    };
  }

  private makeExtraCardType2() {
    this.question = {
      question: "Some question",
      answers: [
        {text: "Answer 1", action: () => {this.action1Type2();}},
        {text: "Answer 2", action: () => {this.action2Type2();}},
      ]
    };
  }

...

}

What does this code do?

A sequence of questions is shown to the user. Each question has several options. In the end of the sequence we do some action and skip this card.

What is the problem?

In this example I have 2 types of extra cards, but I want to have more (5, 10, 20, etc). In this case my component code will be growing too quickly.

What I want

I want to separate the logic of different question sequences. But I'm facing a problem with dependency injection for some unique actions in sequences. And I want to have a single component and use it like this:

<extra-card [type]="type_1"></extra-card>

Also, I want to avoid excessive dependency injections in the base component (in case with passing DI instances to question model).

Not the Strategy yet

I'd try to not have a super huge ExtraCard component class if possible. The idea is to encapsulate the data in a Card interface/classes:

export interface Card {
  context: Context;
  question: string;
  answers: Answer[];
}

export interface Answer {
  text: string;
  action: () => void;
}

export class Card1 implements Card {
  constructor(public context: Context) { }

  question = "Some card";
  answers = [
    { text: "Answer 1", action: () => { console.info("Q1-A1"); this.context.ga.doSomething(); } },
    { text: "Answer 2", action: () => { console.info("Q1-A2"); } },
    { text: "Answer 3", action: () => { console.info("Q1-A3"); } }
  ];
}

export class Card2 implements Card {
  constructor(public context: Context) { }

  question = "Some card";
  answers = [
    { text: "Answer 1", action: () => { console.info("Q2-A1"); } },
    { text: "Answer 2", action: () => { console.info("Q2-A2"); } }
  ];
}

Please notice two things here:

the Card is now responsible for holding the data related to the specific Question (both, question and the list of answer with corresponding behavior). This is not really the "Strategy Pattern" yet, but it is a first big step towards it. In Strategy pattern you normally do not mix the data and behavior in the same class, but rather separate them.
We declare a context: Context property that will hold the important dependencies (aka Services) that are required for a given Card to do it's work in answers' actions.
This allows the card classes to stay regular TypeScript classes instead of being declared as services. This also means that the Context with the services should be provided from the outside. In TypeScript we can use the constructor (public context: Context) to enforce client pass in the context.

The Context, by the way can look something like this:

export class Context {
  ga: GoogleAnalytics;
  socialSharing: SocialSharing;
  appRateService: AppRateService;
  // Other services/dependencies...
}

Now we need to change the ExtraCard component as well as it consumer.

Why not make the ExctraCard as dumb as we can? Here's what it could look like. Notice that the component now expects the entire Card to be passed in via @Input() card. It will also notify the consumer about receiving an answer (see EventEmitter.

{{card.question}}

{{ answer.text }}

import { Component, EventEmitter, Input, Output } from '@angular/core';

@Component({
  selector: 'extra-card',
  templateUrl: 'extra-card.component.html'
})
export class ExtraCard {

  @Input() isActive: boolean = true;
  @Output() answerReceived = new EventEmitter<void>();

  @Input() card: Card;

  triggerAction(answer: Answer): void {
    answer.action();
    this.answerReceived.emit();
  }
}

The last part is your consumer component. It orchestrates Card objects creation and iteration.

The collection of allCards is created in ngOnInit rather than constructor, and it defines the number, kind, and order of the Cards. Consumer's constructor is used to inject the dependencies and prepare a sharedContext that is than passed to the Cards (remember, they now require it in their constructors).

answerReceived() method is used to react to the corresponding event. In my code I assumed that we only need to iterate through the questions linearly (first to last). If that is not the case (e.g. you may need to jump from Card1 to Card3 if a specific Answer was selected), then ExtraCard would need to emit event with some data. For example, answerReceived = new EventEmitter<number>() and this.answerReceived.emit(answer.id). Then the answerId could be used in Consumer for implementing more complex iteration logic.

  <extra-card
    [isActive]="isActive(card)"
    [question]="card"
    (answerReceived)="answerReceived(cardIndex)"></extra-card>

import { Component } from '@angular/core';
import { Card1, Card2, Context, Card } from './extra-card.component';

@Component({
  selector: 'consumer',
  templateUrl: 'consumer.component.html'
})
export class Consumer {

    private sharedContext: Context;
    allCards: Card[] = [];
    activeCard: Card;

    constructor(ga: GoogleAnalytics, appRateService: AppRateService, socialSharing: SocialSharing) {
        this.sharedContext = {
            appRateService,
            ga,
            socialSharing
        };
    }

    ngOnInit(): void {
        this.allCards = [
            new Card1(this.sharedContext),
            new Card2(this.sharedContext),
            // ...
        ];

        this.activeCard = this.allCards[0];
    }

    isActive(card: Card): boolean {
        return this.activeCard === card;
    }

    answerReceived(cardIndex: number): void {
        const card = this.allCards[cardIndex + 1];
        if (card != null) {
            this.activeCard = card;
        } else {
            // TODO Handle card enumeration complete event.
        }
    }
}

Disclaimer

This is by no means the best approach I recommend to use generally, but I think it's better what you have now. Especially, since you identified a very important problem of Card/Question quantity growth. My solution is much more scalable, but it too has its limits.

If you apply this for now, and see some other issues coming, feel free to post  a follow-up question with a specific ask.

I hope this helps.


Calculate the path sum of a binary tree I want to calculate all sum of all possible paths from root to leaf. Each node has value of an integer number (decimal).

In the following example:

    1
   / \
  2   4
 /    /\
3    5  6

Possible paths are *123, 145, and 146, which adds to 414). In this example, 12 is not a valid path since 2 is not a leaf node.

I did basic tests and it works, and I'm wondering if the code is logically correct, and how I can improve its performance.

class Node:

    def __init__(self, value, left, right):
        self.value = value
        self.left = left
        self.right = right

def sum(node, prefix):
    r1 = 0
    r2 = 0
    if not node:
        return
    if node and not node.left and not node.right:
        return prefix*10+node.value
    if node.left:
        r1 = sum(node.left, prefix*10 + node.value)
    if node.right:
        r2 = sum(node.right, prefix*10 + node.value)

    return r1+r2

if __name__ == "__main__":

    node3 = Node(3, None, None)
    node5 = Node(5, None, None)
    node6 = Node(6, None, None)
    node4 = Node(4, node5, node6)
    node2 = Node(2, node3, None)
    node1 = Node(1, node2, node4)

    print sum(node1, 0)

The code can be simplified a bit.

In the sequence:

if not node:
    return
if node and not node.left and not node.right:
    return prefix*10+node.value

...the if node part of the second condition is redundant. Since this is preceded by if not node: return, the second condition will never be reached unless if node evaluates to true.

For the if not node.left and not node.right, I'd apply DeMorgan's theorem, to get if node.left or node.right, and switch the controlled statement with the other.

By having it return 0 for an empty tree as @Zondo suggested, we can simplify a bit more: rather than checking whether node is there before summing it, we can just sum it, and if nothing is there, it returns 0, which doesn't change the sum.

With those and @Zondo's suggestions, we end up with something like this for the sum routine:

def sum(node, prefix = 0):

    if not node:
        return 0

    p = prefix * 10 + node.value

    if node.left or node.right:
        return sum(node.left, p) + sum(node.right, p)

    return p

Another possibility that might be worth considering would be to get a little closer to the single-entry, single-exit "ideal" that used to be advocated in structured-programming:

def sum(node, prefix = 0):

    if not node:
        return 0

    p = prefix * 10 + node.value

    if node.left or node.right:
        p = p + sum(node.left, p) + sum(node.right, p)

    return p

It would be pretty easy to transform this into a true single-entry, single-exit form, but I don't see it as a real improvement (and even what I've done above might be open to some question).


Substring match within a text for given keywords Goal:
Look for a substring match within a text for given Keywords. Keep the substring match if the levenshtein distance is smaller than the length of the Keyword divided by (x=10), else return an empty list.
(First of all my apologies if the code is of lower quality. I am quite good in R but very new to Python.)
Current implementation:
from fuzzywuzzy import process
from fuzzysearch import find_near_matches
import math

def fuzzy_extract(qs, ls, threshold, max_dist):
    ret = []
    for word, _ in process.extractBests(qs, (ls,), score_cutoff = threshold):
        for match in find_near_matches(qs, word, max_l_dist = max_dist):
            word = word[match.start:match.end]
            ret.append({"kw": qs, "word": word, "dist": match.dist})
    return(ret)

def get_match(text, keywords, treshold):
    keywords = [keyword.lower() for keyword in keywords]
    text = text.lower()

    candidates = []
    for li in [fuzzy_extract(query_string, text, 0, 1) for query_string in keywords]:
        if len(li) > 0:
            for di in li:
                if di["dist"] <= math.ceil(len(di["word"])/treshold):
                    candidates.append(di)

    if(len(candidates) == 0):
        return([])

    out = str(candidates[0]["kw"])
    return(out)

keywords = ["apple", "banana", "cherry"]
text = "nana is Looking for an aple."

print(get_match(text, keywords, 10))

For me the part in the middle looks neither efficient and also not very declarative.
With part in the middle i mean the Code following:
for li in [fuzzy_extract(query_string, text, 0, 1) for query_string in keywords]:

I started with something longer but more declarative:
candidates = [fuzzy_extract(query_string, ocr_text, 0, 1) for query_string in keywords_ordering]
lens = [len(candidate) > 0 for candidate in candidates]
candidate_lengths = list(compress(candidates, lens))
filtered = [candidate_length[0]["dist"] < math.ceil(len(candidate_length[0]["word"])/10) for candidate_length in candidate_lengths]
candidate_filtered = list(compress(candidate_lengths, filtered))

In R i would make usage of pipes for These cases to avoid the undeclarative variables in between.
Similar Topics:
Find a best fuzzy match for a string
(Difference is that in this question match candidates are compared to a single word and to a substring of a text).
https://stackoverflow.com/a/36132391/3502164
Custom implementation, rather long.

Unnecessary parentheses
Switch the following:
return(...) -> return ...
if(...) -> if ...

You switch between the two, probably due to coming from another language. Just try to be consistent and don't use parentheses if you don't have to.
Type Hints
You can use type hints to display what types of parameters are accepted, and what types of values are returned, if any.
def fuzzy_extract(query_string: str, match_string: str, threshold: int, max_dist: int):
def get_match(text: str, keywords: List[str], threshold: int):

The List is imported by from typing import List
Simplify your loops
Instead of checking if len(li) > = 0, you can do that check inside the list comprehension. Also, I would move that outside of the loop into its own variable.
extracted = [fuzzy_extract(query_string, text, 0, 1) for query_string in keywords if len(query_string) > 0]

Empty lists
Instead of checking if the length of the list is 0, simply check if the list is None. An empty list is implicitly None.
if not candidates:

Default Parameters
When passing default parameters, there shouldn't be a space before or after the =.
for word, _ in process.extractBests(query_string, (match_string,), score_cutoff=threshold):
        for match in find_near_matches(query_string, word, max_l_dist=max_dist):

Final Code
from fuzzywuzzy import process
from fuzzysearch import find_near_matches
import math

from typing import List

def fuzzy_extract(query_string: str, match_string: str, threshold: int, max_dist: int):
    matches = []
    for word, _ in process.extractBests(query_string, (match_string,), score_cutoff=threshold):
        for match in find_near_matches(query_string, word, max_l_dist=max_dist):
            word = word[match.start:match.end]
            matches.append({"kw": query_string, "word": word, "dist": match.dist})
    return matches

def get_match(text: str, keywords: List[str], threshold: int):
    keywords = [keyword.lower() for keyword in keywords]
    text = text.lower()

    canidates = []
    extracted = [fuzzy_extract(query_string, text, 0, 1) for query_string in keywords if len(query_string) > 0]
    for item in extracted:
        for di in item:
            if di["dist"] <= math.ceil(len(di["word"]) / threshold):
                canidates.append(di)

    if not canidates:
        return []

    result = str(canidates[0]["kw"])
    return result

keywords = ["apple", "banana", "cherry"]
text = "nana is Looking for an aple."

print(get_match(text, keywords, 10))


Parameterized Query in Access SQL I am a beginner in Access SQL. I am in need of writing a parameter query for Access SQL and found a few tutorials and links for that but that wouldn't satisfy my requirement.

DB: Access DB

This updates a 'PD' column based on the values of 'OG', 'CPTY-BA' in an INPUT table. I need to calculate PD values from other reference tables 'PD', 'PD_MASTER_CPTY' and 'PD_MASTER_BG'.

Below is the VBA code with Access SQL with provided parameters. Is there any other way to write it in a single SELECT statement?

ElseIf (outputColumn = "PD") Then

strSQL = "SELECT [OG], CIF,[CPTY-BA], CIF_2 FROM INPUT   WHERE RUN_NUMBER='" + runNumber + "' ORDER BY CIF"
Set rs = CurrentDb.OpenRecordset(strSQL)

If Not (rs.EOF And rs.BOF) Then

     Do Until rs.EOF

        strOrigGroup = rs![Original Group]
        strCIF = rs![CIF]
        strCIF2 = rs![CIF_2]
        strBaselAsset = rs![CPTY-BA]

        If (strOrigGroup = "CB" Or strOrigGroup = "FID" Or strOrigGroup = "PB") Then

                strSQL1 = " SELECT DISTINCT  [INPUT].RUN_NUMBER, [INPUT].CIF, INPUT.CIF_2,[PD].[MID_PD] FROM [PD] INNER JOIN [INPUT] " + _
                          "ON [PD].CIF=[INPUT].CIF_2  WHERE [PD].[CIF]='" + strCIF2 + "' and [INPUT].RUN_NUMBER='" + runNumber + "'"
                Debug.Print strSQL1
                Set rs1 = CurrentDb.OpenRecordset(strSQL1)

                If Not (rs1.EOF And rs1.BOF) Then
                    strPD = CDbl(rs1![MID_PD])
                Else
                    strPD = "0.0229"
                End If

                strSQL = "update output set [PD] = '" + Format(strPD, "#.0000000000000") + "'  WHERE [OUTPUT].[CIF]='" + CStr(strCIF) + "' AND [OUTPUT].RUN_NUMBER='" + runNumber + "'"
                Debug.Print strSQL
                CurrentDb.Execute (strSQL)

        ElseIf (strBaselAsset = "BK" Or strBaselAsset = "BO" Or strBaselAsset = "SK") Then

                strSQL1 = "SELECT DISTINCT [INPUT].RUN_NUMBER,  [INPUT].CIF, [PD_MASTER_CPTY].[PD] FROM PD_MASTER_CPTY INNER JOIN INPUT ON [PD_MASTER_CPTY].COUNTERPARTY=INPUT.[CPTY-BA] WHERE [PD_MASTER_CPTY].[COUNTERPARTY]='" + strBaselAsset + "'AND [INPUT].CIF_2='" + CStr(strCIF2) + "' and [INPUT].RUN_NUMBER='" + runNumber + "'"
                Debug.Print strSQL1
                Set rs1 = CurrentDb.OpenRecordset(strSQL1)

                If Not (rs1.EOF And rs1.BOF) Then
                    strPD = CDbl(rs1![PD])
                Else
                    strPD = "0.0229"
                End If

                strSQL = "update output set [PD] = '" + Format(strPD, "#.0000000000000") + "'  WHERE [OUTPUT].[CIF]='" + CStr(strCIF) + "' AND [OUTPUT].RUN_NUMBER='" + runNumber + "'"
                Debug.Print strSQL
                CurrentDb.Execute (strSQL)

        Else
                strSQL1 = "SELECT [INPUT].RUN_NUMBER, [INPUT].CIF,[PD_MASTER_BG].[PD] FROM [PD_MASTER_BG] INNER JOIN INPUT ON [PD_MASTER_BG].[BUSINESS_GROUP]=INPUT.[ORIGINAL GROUP] WHERE [PD_MASTER_BG].[BUSINESS_GROUP]='" + strOrigGroup + "'AND [INPUT].CIF_2='" + CStr(strCIF2) + "' and [INPUT].RUN_NUMBER='" + runNumber + "'"
                Debug.Print strSQL1
                Set rs1 = CurrentDb.OpenRecordset(strSQL1)

                If Not (rs1.EOF And rs1.BOF) Then
                    strPD = CDbl(rs1![PD])
                Else
                    strPD = "0.0229"
                End If

                strSQL = "update output set [PD] = '" + Format(strPD, "#.0000000000000") + "'  WHERE [OUTPUT].[CIF]='" + CStr(strCIF) + "' AND [OUTPUT].RUN_NUMBER='" + runNumber + "'"
                Debug.Print strSQL
                CurrentDb.Execute (strSQL)

        End If

        rs.MoveNext
        Loop

End If

Before we can do anything here really, we need to clean this up so we can understand exactly what's going on.

To start with, let's remove some duplication. This code is copy/pasted all over the place, but no matter what happens, it executes.

If Not (rs1.EOF And rs1.BOF) Then
    strPD = CDbl(rs1![PD])
Else
    strPD = "0.0229"
End If

strSQL = "update output set [PD] = '" + Format(strPD, "#.0000000000000") + "'  WHERE [OUTPUT].[CIF]='" + CStr(strCIF) + "' AND [OUTPUT].RUN_NUMBER='" + runNumber + "'"
Debug.Print strSQL
CurrentDb.Execute (strSQL)

    If (strOrigGroup = "CB" Or strOrigGroup = "FID" Or strOrigGroup = "PB") Then

            strSQL1 = " SELECT DISTINCT  [INPUT].RUN_NUMBER, [INPUT].CIF, INPUT.CIF_2,[PD].[MID_PD] FROM [PD] INNER JOIN [INPUT] " + _
                      "ON [PD].CIF=[INPUT].CIF_2  WHERE [PD].[CIF]='" + strCIF2 + "' and [INPUT].RUN_NUMBER='" + runNumber + "'"
            Debug.Print strSQL1
            Set rs1 = CurrentDb.OpenRecordset(strSQL1)

    ElseIf (strBaselAsset = "BK" Or strBaselAsset = "BO" Or strBaselAsset = "SK") Then

         strSQL1 = "SELECT DISTINCT [INPUT].RUN_NUMBER,  [INPUT].CIF, [PD_MASTER_CPTY].[PD] FROM PD_MASTER_CPTY INNER JOIN INPUT ON [PD_MASTER_CPTY].COUNTERPARTY=INPUT.[CPTY-BA] WHERE [PD_MASTER_CPTY].[COUNTERPARTY]='" + strBaselAsset + "'AND [INPUT].CIF_2='" + CStr(strCIF2) + "' and [INPUT].RUN_NUMBER='" + runNumber + "'"
         Debug.Print strSQL1
         Set rs1 = CurrentDb.OpenRecordset(strSQL1)
    Else
        strSQL1 = "SELECT [INPUT].RUN_NUMBER, [INPUT].CIF,[PD_MASTER_BG].[PD] FROM [PD_MASTER_BG] INNER JOIN INPUT ON [PD_MASTER_BG].[BUSINESS_GROUP]=INPUT.[ORIGINAL GROUP] WHERE [PD_MASTER_BG].[BUSINESS_GROUP]='" + strOrigGroup + "'AND [INPUT].CIF_2='" + CStr(strCIF2) + "' and [INPUT].RUN_NUMBER='" + runNumber + "'"
        Debug.Print strSQL1
        Set rs1 = CurrentDb.OpenRecordset(strSQL1)
    End If

    If Not (rs1.EOF And rs1.BOF) Then
        strPD = CDbl(rs1![PD])
    Else
        strPD = "0.0229"
    End If

    strSQL = "update output set [PD] = '" + Format(strPD, "#.0000000000000") + "'  WHERE [OUTPUT].[CIF]='" + CStr(strCIF) + "' AND [OUTPUT].RUN_NUMBER='" + runNumber + "'"
    Debug.Print strSQL
    CurrentDb.Execute (strSQL)

    rs.MoveNext
Loop

Next, give your SQL statement variables some meaningful names. I shouldn't have to keep reminding myself that strSQL1 is the select statement and strSQL is the update. Try selectSQL and updateSQL respectively.

Next, I would separate the messy business of creating the string to be executed from the business of actually executing.

Private Function BuildSelectStatement(byval cif as string, byval runNumber as integer, Optional byval baselAsset as String) As String
    ...
End Function

....

selectSQL = BuildSelectStatement(strCIF2, runNumber)

Which brings me to recommending that you use ADODB for this. The only time it's really important to stick with DAO is when you intend on binding a form to the resulting Recordset. Instead of concatenating all of these parameters into strings, you could instead be passing parameter into parameterized queries. It looks and reads much better than this.


Move and rename images into separate folders Basically, we came up with this task at work. I was wondering if there's a shorter and cleaner way of doing the following in some other language (or even improving what I did with Ruby).

We have a folder. In this folder we have files (images only, /w .jpg extension). The names of the images are random strings such as:

3e3eaa0b927d0158e1b2479ce31000b3.jpg
3e3eaa0b927d0158e1b2479ce31000b3_1.jpg
99a2b9c3442e716fc486e46abcfcfb05.jpg
99a2b9c3442e716fc486e46abcfcfb05_1.jpg
99a2b9c3442e716fc486e46abcfcfb05_2.jpg & etc.

To make it clearer, those images were related to certain objects. An object had multiple images (not a specific amount). We have to get those images, read their name, and get the "unique" part.

Example: For images:

3e3eaa0b927d0158e1b2479ce31000b3.jpg
3e3eaa0b927d0158e1b2479ce31000b3_1.jpg

the "unique" part is "3e3eaa0b927d0158e1b2479ce31000b3"

And we have to create a folder with this name (3e3eaa0b927d0158e1b2479ce31000b3) and put all the images that match (3e3eaa0b927d0158e1b2479ce31000b3_*). Also we have to rename those images, so in every folder there must be one image with thumb_ followed by a random string & the other images must be with img_ prefix and again followed by a random string.

This is my try:

require 'securerandom'
folder = "storage_test/"
Dir.glob("#{folder}*.jpg") do |img|
  fn = File.basename(img,".*").gsub(/(\_\d)+/, '') #folder name
  prefix = (!File.exists?( "#{folder}#{fn}") ? '/thumb_' : '/img_') #prefix
  new_name = "#{folder}#{fn}/#{prefix}" + SecureRandom.hex(32) + File.extname(img) #new_name (of the image)
  Dir.mkdir("#{folder}#{fn}")  unless File.exists?("#{folder}#{fn}") #create the folder
  File.rename(img, new_name) #move
end

(I work on Windows)

I'm sorry I've probably misused the term random. By random I mean that I need to create a name that's unique to the certain folder. Actually, there aren't going to be more than 40-50 files in a folder, so I presumed that SecureRandom.hex(32) will generate unique enough filename that will not cause overwriting in the same folder.

More detailed example:

Example folder: "storage_test"

And this is a part of its content:

I've grouped the files that should go in the same folder. I highlighted that part from the file's name that should be used as folders name.

So after executing the code "storage_test" will have:

091afecdc49a8e8e581f11f2f1bfea8f_1/thumb_unique(to the folder)name.jpg
091afecdc49a8e8e581f11f2f1bfea8f_1/img_unique(to the folder)name.jpg
091afecdc49a8e8e581f11f2f1bfea8f_1/img_unique(to the folder)name.jpg

As mentioned in the comments, your code is pretty dense. A little whitespace would help.

Your code comments also seem a little off. Many could be avoided by simply using more descriptive names for things. And a comment like prefix = ... #prefix doesn't really help. I also wonder why a comment like #new name (of the image) has a parenthetical. It's like the comment has a comment.

As for the functionality, your code seems ok, but there's one potentially fragile bit: Deciding the file prefix. You check for the existence of a directory when deciding the prefix. If the directory exists, you assume it also contains a thumb_* file. But that's not necessarily given. It'd be better to check if there's a thumb_* file there.

I'd also use File.join for more robust path contatenation.

In general, I'd break the logic into a few methods to make the steps more explicit. I'm making two assumptions here: That the images are always ".jpg", and that destination directories will be in the source directory.

require "securerandom"

# Gets the leading hexadecimal part of a filename
def unique_id(filename)
  $1 if File.basename(filename) =~ /\A(\h+)/
end

# Is there a file named "thumb_*.jpg" in the given directory?
def thumbnail_exists?(directory)
  pattern = File.join(directory, "thumb_*.jpg")
  Dir[pattern].any?
end

# Get the destination directory for a file
def directory_for_file(filename)
  base = File.dirname(filename)
  id = unique_id(filename)
  File.join(base, id)
end

# Returns the path for where to move an image file
def destination_for_file(filename)
  directory = directory_for_file(filename)
  prefix = thumbnail_exists?(directory) ? "img" : "thumb"
  new_name = "#{prefix}_#{SecureRandom.hex(32)}.jpg"
  File.join(directory, new_name)
end

# Move files
source_pattern = File.join("storage_test", "*.jpg")
Dir[source_pattern] do |file|
  directory = directory_for_file(file)
  Dir.mkdir(directory) unless File.exists?(directory)
  new_path = destination_for_file(file)
  File.rename(file, new_path)
end

It's kinda overwrought for what it has to do, but that's sort of on purpose to present a starker alternative.


Stack Overflow user info fetcher I'm starting to look at Python and its facilities. I've just tried to make one simple program using some base libraries.

I would like to hear some comments about anything you've noticed - style, readability, safety, etc.

#!/usr/bin/python2.6 -u

import urllib2
import simplejson
import zlib
from optparse import OptionParser

class StackOverflowFetcher:
    """Simple SO fetcher"""
    def getUserInfo( self, userId ):
        response = urllib2.urlopen( 'http://api.stackoverflow.com/1.1/users/' + str(userId) )
        response = response.read()
        jsonData = zlib.decompress( response, 16+zlib.MAX_WBITS )
        return simplejson.loads( jsonData )

    def getUserDisplayName( self, userId ):
        return self.getUserInfo( userId )['users'][0]['display_name']

    def getUserViewCount( self, userId ):
        return self.getUserInfo( userId )['users'][0]['view_count']

    def getUserReputation( self, userId ):
        return self.getUserInfo( userId )['users'][0]['reputation']

if __name__ == '__main__':
    parser = OptionParser()
    parser.add_option("-u", "--userId", dest="userId", help="Set userId (default is 1)", default=1 )
    parser.add_option("-n", "--display-name", action="store_true", dest="show_display_name", default=False, help="Show user's display name")
    parser.add_option("-v", "--view_count", action="store_true", dest="show_view_count", default=False, help="Show user's profile page view count")
    parser.add_option("-r", "--reputation", action="store_true", dest="show_reputation", default=False, help="Show user's reputation")
    (options, args) = parser.parse_args()

    userId = options.userId
    show_display_name = options.show_display_name
    show_view_count = options.show_view_count
    show_reputation = options.show_reputation

    if ( (not show_display_name) and (not show_view_count) and (not show_reputation) ):
        show_display_name = show_view_count = show_reputation = True

    fetcher = StackOverflowFetcher()
    if ( show_display_name ) : print fetcher.getUserDisplayName( userId )
    if ( show_view_count) : print fetcher.getUserViewCount( userId )
    if ( show_reputation ) : print fetcher.getUserReputation( userId )

Python version 2.6

I think your naming scheme is good and on first sight its very easy to see what the code does without having to study it too much.

However, if I would pick on something I would say:

Remove the unnecessary whitespace, ex. if ( condition ), or self.getUserInfo( userId ). This is of course a matter of taste, but I find it more consistent with regular coding style to not 'bloat' with whitespace.
optparser was deprecated in 2.7: http://docs.python.org/library/optparse.html
in parser.add.option() you could remove the word "show" from the dest name and rather do the following, hence eliminating the need for all the declarations of show_display_name and still keep easy code readability.

...
parser.add.option(parser.add_option("-r", "--reputation", action="store_true", dest="reputation", default=False, help="Show user's reputation")
(show, args) = parser.parse_args()

fetch = StackOverflowFetcher()
if(show.reputation) : print fetch.getUserReputation(show.userId)

I don't really see what the point of the following line which sets everything to true, it seems a bit weird since you use store_true as action if the option parsed is set.

if ( (not show_display_name) and (not show_view_count) and (not show_reputation) ):
show_display_name = show_view_count = show_reputation = True


Making Anagram Problem Implementation Problem Statement

Alice is taking a cryptography class and finding anagrams to be very useful. We consider two strings to be anagrams of each other if the first string's letters can be rearranged to form the second string. In other words, both strings must contain the same exact letters in the same exact frequency. For example, bacdc and dcbac are anagrams, but bacdc and dcbad are not.
Alice decides on an encryption scheme involving two large strings where encryption is dependent on the minimum number of character deletions required to make the two strings anagrams. Can you help her find this number?
Given two strings, a and b, that may or may not be of the same length, determine the minimum number of character deletions required to make a and b anagrams. Any characters can be deleted from either of the strings.

Solution
//Valid Test Case For Reference
static void ransomeNoteTrue() {
    String a = "cde";
    String b = "abc";
    int result = makeAnagram(a, b);
    int expected = 4;
    assert (result == expected);
}

static int makeAnagram(String a, String b) {
    Character[] aArray = a.chars().mapToObj(x -> (char) x).toArray(Character[]::new);
    Character[] bArray = b.chars().mapToObj(x -> (char) x).toArray(Character[]::new);
    Map<Character, Long> aMap = getFrequencyMapFromArray(aArray);
    Map<Character, Long> bMap = getFrequencyMapFromArray(bArray);

    return countError(aMap, bMap);
}

private static int countError( Map<Character, Long> aMap, Map<Character, Long> bMap ) {
    long deletedChars = 0;

    for( char ch='a'; ch<='z'; ch++) {

        if ( aMap.containsKey(ch) && !bMap.containsKey(ch) )
            deletedChars += aMap.get(ch);

        else if( bMap.containsKey(ch) && !aMap.containsKey(ch) )
            deletedChars += bMap.get(ch);

        else if( bMap.containsKey(ch) && aMap.containsKey(ch) )
            deletedChars += Math.abs( aMap.get(ch) - bMap.get(ch));
    }

    return (int)deletedChars;
}

private static Map<Character, Long> getFrequencyMapFromArray(Character[] arr) {
    return Arrays.stream(arr)
            .collect(Collectors.groupingBy(Function.identity(), Collectors.counting()));
}

The main function to review is makeAnagram().
Looking forward to your reviews on -

What other improvements can be done?
What other ways could be there to the problem, as the current
approach is brute force?

Thank you.

Do not map to array

I appreciate that streaming strings is a little messy. However, there is no need to create an array; instead we can just transform the stream. I would remove the arrays and change getFrequencyMap to take a String.

Use Map::getOrDefault to simplify countError

In particular, if you do aMap.getOrDefault(ch, 0) then the Math.abs case always works.

Unlike @DapperDan's solution, this still requires looping through all 26 characters; however, the logic is a lot simpler. At least on my system, my approach is faster, even in you example when each map has only 3 elements. The cost of iterating through 26 characters is very low compared to map operations.

Use of longs

I notice you use Longs throughout (presumably due to Collectors.counting() returning a Long) but then cast to int at the end. Why not just return a long?

Stream rangeClosed vs for loop

Mostly a matter of style, but it is easy to replace the for loop with a stream.

Code

Applying these changes should yield something like

static long makeAnagram(String a, String b) {
    return countError(getFrequencyMap(a), getFrequencyMap(b));
}

private static long countError(Map<Character, Long> aMap, Map<Character, Long> bMap) {
    return IntStream.rangeClosed('a', 'z')
        .mapToObj(x -> Character.valueOf((char)x))
        .mapToLong(c -> Math.abs(aMap.getOrDefault(c, 0l) - bMap.getOrDefault(c, 0l)))
        .sum();
}

private static Map<Character, Long> getFrequencyMap(String s) {
    return s.chars()
        .mapToObj(x -> Character.valueOf((char)x))
        .collect(Collectors.groupingBy(Function.identity(), Collectors.counting()));
}


Showing segue after button action I have an iOS Swift app with 8 buttons that call the same segue, but this is very repetitive. How can I refactor so it uses the same function prepare?

@IBAction func backButtonPressed(_ sender: Any) {
    self.vcWithType = VCtype.back.rawValue
    self.postion = "back"
    self.performSegue(withIdentifier: "showCategory", sender: sender)
}

@IBAction func mountButtonPressed(_ sender: Any) {
    self.vcWithType = VCtype.mount.rawValue
    self.postion = "mount"
    self.performSegue(withIdentifier: "showCategory", sender: sender)
}

@IBAction func kneeRideButtonPressed(_ sender: Any) {
    self.vcWithType = VCtype.knee.rawValue
    self.postion = "knee"
    self.performSegue(withIdentifier: "showCategory", sender: sender)
}

@IBAction func sideKbuttonPressed(_ sender: Any) {
    self.vcWithType = VCtype.sideK.rawValue
    self.postion = "side"
    self.performSegue(withIdentifier: "showCategory", sender: sender)
}

@IBAction func northSouthBpressed(_ sender: Any) {
    self.vcWithType = VCtype.northSouth.rawValue
    self.postion = "north"
    self.performSegue(withIdentifier: "showCategory", sender: sender)
}

@IBAction func guardBpressed(_ sender: Any) {
    self.vcWithType = VCtype.guarda.rawValue
    self.postion = "guard"
    self.performSegue(withIdentifier: "showCategory", sender: sender)
}

@IBAction func halfGuardBpressed(_ sender: Any) {
    self.vcWithType = VCtype.halfGuard.rawValue
    self.postion = "half"
    self.performSegue(withIdentifier: "showCategory", sender: sender)
}

@IBAction func turtleBpressed(_ sender: Any) {
    self.vcWithType = VCtype.turtle.rawValue
    self.postion = "turtle"
    self.performSegue(withIdentifier: "showCategory", sender: sender)
}

override func prepare(for segue: UIStoryboardSegue, sender: Any?) {

    if (segue.identifier == "showCategory") {

        if let vc: CategoryViewController = segue.destination as? CategoryViewController {
            vc.vcWithType = self.vcWithType
            vc.chaza = self.postion

        }

    }

}

I think easiest way is using tags.
You can connect more then one button to one IBAction func. Just Make sure sender is UIButton.

After that change buttons tag property.

After changing tags. We just need to use switch-case

@IBAction func ButtonAction(_ sender: UIButton) {
      switch sender.tag {
            case 0:
                print("Button 1 Clicked")
            case 1:
                print("Button 2 Clicked")
            default: break
      }
}


Python Prefix Notation Evaluater I'm designing a new esoteric language, and the interpreter is in Python3. It is going to use Prefix Notation, so as part of the interpreter, I need a function that evaluates a string as prefix notation and returns a number. Here is my code so far:

import re
import math

class Operator:
    def __init__(self, arity, function):
        self.arity = arity
        self.function = function

    def __call__(self, *args):
        assert(len(args) == self.arity)
        return self.function(*args)

operatorTable = { "+": Operator(2, lambda a,b: a+b),
             "-": Operator(2, lambda a,b: a-b),
             "/": Operator(2, lambda a,b: a/b),
             "*": Operator(2, lambda a,b: a*b),
             "<": Operator(2, lambda a,b: a<b),
             ">": Operator(2, lambda a,b: a>b),
             "_": Operator(1, lambda a: -a),
             "p": Operator(0, lambda: math.pi),
             "e": Operator(0, lambda: math.e)
}

def prefixParse(code):
    if code[0] in operatorTable:
        operator = operatorTable[code[0]]

        if operator.arity == 2:
            arg1, chomp1 = prefixParse(code[1:])
            arg2, chomp2 = prefixParse(code[chomp1 + 1:])
            return operator(arg1, arg2), chomp1 + chomp2 + 1

        elif operator.arity == 1:
            arg, chomp = prefixParse(code[1:])
            return operator(arg), chomp + 1

        elif operator.arity == 0:
            return operator(), 1

    match = re.match("^\d+", code)
    if match:
        return int(match.group(0)), match.end()

    if code[0] == ' ':
        res = prefixParse(code[1:])
        return res[0], res[1] + 1

print(prefixParse("+2*5 10")[0])

I'm interested in hearing any improvements you have to offer, but I'm particularly interested in these design questions:

Is the recursive prefixParse a good/suitable algorithm for parsing prefix notation, or is there a simpler way to do it?
Is it overkill to use the Operator class?
Is there a way to "chomp" the string in place rather than handing a bunch of string indices back and forth? That would be nice because then to get an integer result from the parse function, I don't have to do prefixParse(code)[0] which looks kinda ugly IMO.
Is the layout of the multi-line operatorTable assignment good code formatting?

Only addressing your last point about the layout of operatorTable, i would use 4 space indentation as according to PEP8-indentation.

operatorTable = {
    "+": Operator(2, lambda a,b: a+b),
    "-": Operator(2, lambda a,b: a-b),
    "/": Operator(2, lambda a,b: a/b),
    "*": Operator(2, lambda a,b: a*b),
    "<": Operator(2, lambda a,b: a<b),
    ">": Operator(2, lambda a,b: a>b),
    "_": Operator(1, lambda a: -a),
    "p": Operator(0, lambda: math.pi),
    "e": Operator(0, lambda: math.e),
}

Also note the comma after the last item. This has a small advantage when you use a source control system, when another item is added after the last item only the new line will show up in a diff and not the previous line which only has a comma added.


Is this code thread-safe - Singleton Implementation using Concurrent Dictionary class Connection
{
    private string param1;
    private string param2;
    private static readonly ConcurrentDictionary<Tuple<string, string>, Connection>
        connections = new ConcurrentDictionary<Tuple<string, string>, Connection>();
    private Connection()
    {
    //Prevent instantiation
    }

    private Connection(string param1, string param2)
    {
        this.param1 = param1;
        this.param2 = param2;
    }

    public static Connection getInstance(string param1, string param2)
    {
        Connection conn = activeConnections.GetOrAdd(new Tuple<string, string>
            param1,param2), new Connection (param1, param2));
        return conn;
    }
}

Never create two connection objects with the same parameters. If one exists, use it.

If you really need to guarantee this, then I think you will need to use locking instead of ConcurrentDictionary.

If it's okay to create duplicate Connections (that will never be used) in rare circumstances, then you can use an overload of GetOrAdd() that takes a lambda that creates the Connection:

return activeConnections.GetOrAdd(
    Tuple.Create(param1,param2), _ => new Connection (param1, param2));

With your current code, every time you call getInstance(), a new Connection is created and then most of the time thrown away.


Simple elapsed time counter I've written a simple JavaScript function based off some existing code that takes a specific UNIX time string (in the snippet: 1491685200). It shows an alert if the specified time is still in the future. When that date has passed, it shows the elapsed days, hours and minutes whenever the function is called in-page.

I'm don't have much JavaScript object experience so it works just fine as-is, but I think it can be optimized further before I migrate my other stuff over to ES6.

function ElapsedTime () {
  var nTotalDiff = Math.round((new Date()).getTime() / 1000) - 1491685200;
  if (nTotalDiff >= 0) {
    var oDiff = {};
    oDiff.days = Math.floor(nTotalDiff / 86400);
    nTotalDiff -= oDiff.days * 86400;
    oDiff.hours = Math.floor(nTotalDiff / 3600);
    nTotalDiff -= oDiff.hours * 3600;
    oDiff.minutes = Math.floor(nTotalDiff / 60);
    nTotalDiff -= oDiff.minutes * 60;
    oDiff.seconds = Math.floor(nTotalDiff);
    return oDiff;
  } else {
    alert('nTotalDiff still not >=0, so must be before the specified date');
  }
}

function TimePassed() {
  oDiff = ElapsedTime();
  if(oDiff) {
    alert(oDiff.days + 'd ' + oDiff.hours + 'h ' + oDiff.minutes + 'm ');
  }
}

//   document.getElementById('counter').addEventListener('click', TimePassed, false);

Remarks to your code

Function names should be using camelCase.
Two minor formatting errors: function ElapsedTime (), if(oDiff).
                                               ↑       ↑
You mentioned ES6 in your question, so: var nTotalDiff could be declared using let and var oDiff using const.
You declare oDiff in TimePassed() as a global variable (you forgot var).
oDiff.seconds is declared, but never used.
(new Date()) can be written as new Date().
Specifying 1491685200 in human readable format would be better, I think.Update: suggestion removed due to the problem pointed out in the comment to this answer.
If you leverage power of modulo operator, you can get rid of all these nTotalDiff -= oDiff.██████ * ███.
alert(oDiff.days + 'd ' + oDiff.hours + 'h ' + oDiff.minutes + 'm ') can be written in ES6 using template literals as alert(`${oDiff.days}d ${oDiff.hours}h ${oDiff.minutes}m `), although space after m is unnecessary whatsoever.
Having two functions named ElapsedTime() and TimePassed() seem ambiguous.
Your code is not in strict mode.

My rewrite

const elapsedTime = () => {
  'use strict';
  const since   = 1491685200000, // Saturday, 08-Apr-17 21:00:00 UTC
        elapsed = (new Date().getTime() - since) / 1000;

  if (elapsed >= 0) {
    const diff = {};

    diff.days    = Math.floor(elapsed / 86400);
    diff.hours   = Math.floor(elapsed / 3600 % 24);
    diff.minutes = Math.floor(elapsed / 60 % 60);
    diff.seconds = Math.floor(elapsed % 60);

    let message = `Over ${diff.days}d ${diff.hours}h ${diff.minutes}m ${diff.seconds}s.`;
    message = message.replace(/(?:0. )+/, '');
    alert(message);
  }
  else {
    alert('Elapsed time lesser than 0, i.e. specified datetime is still in the future.');
  }
};

document.getElementById('counter').addEventListener('click', elapsedTime, false);
<button id="counter">Check elapsed time</button>

Remarks

/(?:0. )+/ matches sequences of one or more zero followed by any character and a space. It is used to change potential output that would look like 0d 0h 0m 1s to simply 1s.
In message I added word over, because the calculated time is in fact the least amount of time that could have passed (calculation time, Math.floor(), instantaneous nature of time specification invalidation).


Python: check if all array values are same I have an example array like:

array = [1, 2, 3, 4, 5, 6]

Every of this numbers refers to an object that has text property. I want to make sure that every of these object has the same text value.
Currently I do it like this:

correct = []
for number in array:
  if(object[number].text == "Sample"):
    correct.append(number)
if correct.length == array.length:
  return True

Is there a more efficient way to do this, like with lambda function or?

You are iterating over the list, and copying elements that have a certain property to the correct list, then checking whether it's of the size of the original list. In other words, you're checking whether all elements fulfill a certain condition, in other words:

return all(object[number].text == "Sample" for number in array)


8088 assembly checksum implementation The following 8088/8086 assembly program tests the modular-sum checksum of a block of data. I suspect it could be tighter; can anyone do it in fewer bytes?

31        xor        ax,ax          ; zero the sum
c0
bb        mov        bx,2000h        ; count 8kb of ROM
00
20
4b        dec        bx             ; start at the top
02        add        al,[bx]        ; add the current byte to the total
07
85        test       bx,bx          ; see if we're at the bottom yet
db
75        jnz        -7             ; loop while not done
f9
84        test       al,al          ; sum should be 0
c0
74        jz         1              ; if sum is good, continue with program
01
f4        hlt                       ; if sum is bad, stop

I'm not absolutely certain (I haven't run it through an assembler or checked against an instruction chart), but I believe this may be a byte shorter:

    xor dl, dl
    xor si, si
    mov cx, 2000h
again:
    lodsb       ; load value, increment SI
    add dl, al  ; set z flag based on result of addition
    loop again  ; decrement CX, jump if NZ, *without* affecting flags

    jz good     ; Z flag still has result of addition above
    hlt
good:


Choosing between local and server data I have some working code, but it seems a bit clunky.

I need to get data from a service. The service will hold the data in local storage if possible (to make it speedier), but if it's not been accessed before, it will need an http GET to a web server. The example contains only one GET, but there could be many in the future.

Service code

var module = angular.module('myapp.service', ["ngStorage"]);

module.service('jsonlistservice', function ($http, $q, $timeout, $localStorage) {

    var getStuff = function(){

      if ($localStorage.stuff != undefined && $localStorage.stuff != "") {

        return $q(function (resolve, reject) {
          var x = {};
          x.data = $localStorage.stuff;
          resolve(x);
        });

        }else{
            return $http.get('data/sample.json').success(function(response){
              $localStorage.stuff = response;
            });
        }
    };

    return{
        getStuff: getStuff,
    };
});

From a controller this would be accessed like so:

'use strict';
angular.module('myApp.view1', ['ngRoute', 'myapp.service'])

.config(['$routeProvider', function($routeProvider) {
  $routeProvider.when('/view1', {
    templateUrl: 'view1/view1.html',
    controller: 'View1Ctrl'
  });
}])

.controller('View1Ctrl', ['$scope', 'jsonlistservice', function($scope, myappservice) {

    var myDataPromise = myappservice.getStuff();
        myDataPromise.then(function(result){
            $scope.test = result.data[950].Id;
            console.log("returned");
        });

}]);

What is the correct pattern for this sort of thing please?

Going with your existing code, the promise part can be simplified.

var getStuff = function(){
  return !!$localStorage.stuff ? $q.resolve({ data: $localStorage.stuff })
                               : $http.get('data/sample.json').then(function(response){
                                   $localStorage.stuff = response;
                                   return response;
                                 });
};

You appear to check against undefined and a blank string for stuff, both falsy values. We can simplify by coercing them to a boolean using !!. Another improvement is using $q.resolve, which returns an already resolved promise, resolved value optional. In the same way, $http.get also returns a promise. We can simply just return them instead of creating a new promise wrapper.

myappservice.getStuff().then(function(result){
  $scope.test = result.data[950].Id;
  console.log("returned");
});

We can also chain the then to getStuff since getStuff returns a promise. Much easier to follow chained stuff than breaking them apart (might be personal preference, but chaining makes code appear like sentences instead of commands).

As for a pattern, I think this one is fine since this is a one-off request (call and get) and the controller has been shielded from knowing where the data came from.

However, if you decide to go stream this data in, like say from a websocket, or make your service poll for changes, or react to localStorage events, then the suggested Observer Pattern might be your best bet. Make your service collect callbacks and fire them when something interesting happens.


Selecting indices of a cell array that strcmpi-match the contents of another cell array I have a Matlab structure with several fields, 3 of which are interesting here:

elec.label %Nx1 cell-array of strings
elec.chanpos %Nx3 matrix of doubles
elec.elecpos %Nx3 matrix of doubles

The label contains unique names of channels, while the chanpos and elecpos contain coordinates, in such a way that elec.label(1) has the name of the channel one, and elec.chanpos(1,:) and elec.elecpos(1,:) have the coordinates of channel one.

What I need is to create a similar structure, which is a subset of elec and contains labels and the two coordinate fields, and is based on a list of labels that I have. The labels I have do not necessarily match capitalization used in the elec.label array.

Here's how I implemented this:

list46={'fpz';'afz';};%Real list is really long, so I truncated it.
INDX46=[];
for i=1:length(elec.label)
    for j=1:length(list46)
        if(strcmpi(list46(j),elec.label(i)))
            INDX46=[INDX46; i;];
        end
    end
end

elec46.chanpos=elec.chanpos(INDX46,:);
elec46.elecpos=elec.elecpos(INDX46,:);
elec46.label=elec.label(INDX46,:);

The implementation I did, to me, feels as if I'm trying to program C using Matlab. I'm looking for a more efficient or at least more Matlabesque way to do this.

Appending to an array

INDX46=[INDX46; i;]; is a very slow way of appending to a vector. Instead, do INDX46(end+1) = i;.

When appending to the matrix in this preferred way, MATLAB actually extends the storage of the vector. MATLAB will double the underlying memory block size so that repeated appending will be optimal (O(log n) rather than O(n)). That is, most of the loop iterations the data will not need to be copied to extend the array.

In contrast, the first method of appending actually creates a new array, and copies the old array and the new value into it. The interpreter does not use the optimal O(log n) method of extending the array.

See this Q&A on Stack Overflow for an experiment that demonstrates the above.

Finding strings

There's no data in the OP that can be easily used to test modifications, but as I see it, the double loop is comparing each of the elements of list46 to each of the elements of elec.label to find the indices in the latter that contain an element of the former. This loop can be sped up greatly by first sorting alphabetically the two lists. With two sorted lists, one need to go through both lists only once to find all matches. The algorithm goes from O(nm) to O(n+m), with n and m the lengths of the two arrays.

MATLAB has a function ismember that does just this:

INDX46 = ismember(elec.label, list46);


Will this converstion to/from fixed point cause me to lose precision? I have a situation where I have a fixed point number that I want to convert to and from a floating point number.  Specifically it is the SANE_Fixed type from the SANE API.  Here is the what the documentation says (I grabbed the bits I think are relavant):

  SANE_Fixed is used for variables that can take fixed point values in the range -32768 to 32767.9999 with a resolution of 1/65535.

#define SANE_FIXED_SCALE_SHIFT  16
typedef SANE_Word SANE_Fixed;

  The macro SANE_FIXED_SCALE_SHIFT gives the location of the fixed binary
  point. This standard defines that value to be 16, which yields a
  resolution of 1/65536.

  SANE_Word: A word is encoded as 4 bytes (32
  bits). The bytes are ordered from most-significant to
  least-significant byte (big-endian byte-order).

So, over the wire I get 4 bytes which I convert into a .Net int like this (you can guess what GetByte and SendByte do):

public int GetWord()
{
    int value = 0;
    value += (GetByte() << 24);
    value += (GetByte() << 16);
    value += (GetByte() << 8);
    value += (GetByte() << 0);
    return value;
}

Then I want to convert that int into a floating point number (I went for decimal) like this:

public decimal ToFloating(int source)
{
    decimal value = source / ((decimal)(1 << 16));
    return value;
}

And I will also need to go back the other way so I would convert to fixed like this:

public int ToFixed(decimal source)
{
    decimal value = source * ((decimal)(1 << 16));
    return (int)value;
}

And then convert to four bytes like this to send it like so:

public void SendWord(int word)
{
    SendByte((word >> 24) & 0xff);
    SendByte((word >> 16) & 0xff);
    SendByte((word >> 8) & 0xff);
    SendByte((word >> 0) & 0xff);
}

So, here is my question:

This seems a pretty simplistic implementation to me and I read lots of stuff on the internet about creating custom fixed point classes and whatnot but, given the constraints of my scenario, is this a safe approach to take?  Could I lose precision and is the maths correct?

If the SANE_Fixed type has a constantly fixed precision of 1/65536, that means it always stores the fractional part of the number as a 16 bit (two-byte) unsigned integer (hence the SANE_FIXED_SCALE_SHIFT of 16 bits).  Since the whole number portion of the value can range from -32768 to 32767, that means that the whole part of the number is represented with a 16 bit signed integer (making the type use a total of 4 bytes).

The decimal type in .NET, while it is a floating point type, it is far more precise that the traditional float type.  It stores a 96 bit integer (plus an additional bit for the sign).  It then stores the position of the decimal point as a 5-bit integer, which allows the decimal point to be located after any digit in the 96-bit integer value.  As such, the larger the value gets, the lower the fractional precision becomes.

In other words, the decimal type can store a value up to 79,228,162,514,264,337,593,543,950,335 (29 digits).  If it does store a 29 digit number, however, it will be unable to store any fractional value with that.  It will only store the whole part of the number.  If, however, it is only storing a single digit value, the fractional part can be up to 28 digits long (e.g. 0.0000000000000000000000000001).

Bearing all of that in mind, if the maximum whole number that the SANE_Fixed type can store is 32767 (a five digit number), when cast to a decimal type, that leaves 24 digits of precision (e.g. 1/10^24) in the fractional part of the number.  That is still far more precise than the 1/65536 precision provided by the SANE_Fixed type.

Therefore, you can be confident that the decimal type will accurately retain the precise value.  Obviously, when casting from a decimal back into a SANE_Fixed type, you will lose precision, but since the value came from that type in the first place, your values are safe.


Function takes a dictionary with list as value, returns the key of the biggest list I created a function that takes a dictionary as parameter. This dictionary is expected to have lists as value pair. (i.e. {1: [1, 2, 3], 'a': [4, 'b', 5, 'c']} )
This function has to return the key associated to the biggest list in the dictionary, or None if the dictionary is empty. (considering the above example, it would return 'a' )
I came up with this code:
def biggest(aDict):
    return None if not aDict else [x for x in aDict if len(aDict[x]) == max(len(x) for x in aDict.values())][0]

Can I have some review on this and any ideas on how to further simplify without using external libraries?

Your code is difficult to read:

The line is too long (according to the Python style guide all lines should be limited to 79 characters).
The same iterator variable x is used for the outer iteration and also inside the max() function.

It is also inefficient because the maximal list length is determined in each iteration.
Using the key and default argument of the max() function the same can be concisely and efficiently be implemented as
def biggest(aDict):
    return max(aDict, key=lambda k: len(aDict[k]), default=None)

More suggestions:

Have a look at the Python naming conventions (snake_case, not camelCase).
Choose a more descriptive function name.
Use docstrings to describe the function.


shared_ptr basic implementation for non array types This is an implementation to simulate the basic functionality of c++ shared_ptr.
This doesn't provide features like custom deleter and make_shared().

I would really appreciate any feedback to improve the below code, any other api's that I should be providing etc.

#ifndef SHARED_PTR_H_
#define SHARED_PTR_H_

#include <utility>

namespace kapil {
  template <typename T>
  class shared_ptr {
    private:
      T* ptr_;   // data pointer
      int* ref_count_;  // reference count pointer

      void decrement_ref_count_and_delete_if_needed() {
        if (ref_count_) {
          --(*ref_count_);
            if (*ref_count_ == 0) {
              delete ptr_;
              delete ref_count_;
              ptr_ = nullptr;
              ref_count_ = nullptr;
            }
        }
      }

    public:
      constexpr shared_ptr() noexcept : ptr_{nullptr}, ref_count_{nullptr} {}

      constexpr explicit shared_ptr(T* ptr) {                  // constructor
        ptr_ = ptr;
        ref_count_ = new int{1};
      }

      shared_ptr(const shared_ptr& other) noexcept { // copy constructor
        ptr_ = other.ptr_;
        ref_count_ = other.ref_count_;
        if (ref_count_ != nullptr) {
          ++(*ref_count_);
        }
      }

      shared_ptr(shared_ptr&& other) noexcept {  // move constructor
        ptr_ = other.ptr_;
        ref_count_ = other.ref_count_;
        other.ptr_ = nullptr;
        other.ref_count_ = nullptr;
      }

      ~shared_ptr() noexcept {             // destructor
        decrement_ref_count_and_delete_if_needed();
      }

      shared_ptr& operator = (const shared_ptr& other) noexcept {   // assignent operator
        if (this != &other) {
          decrement_ref_count_and_delete_if_needed();
          ptr_ = other.ptr_;
          ref_count_ = other.ref_count_;
          if (ref_count_) {
            ++(*ref_count_);
          }
        }
        return *this;
      }

      shared_ptr& operator = (shared_ptr&& other) noexcept {   // move assignment operator
        *this = other;
      }

      T* get() const noexcept {
        return ptr_;
      }

      void reset() noexcept {
        decrement_ref_count_and_delete_if_needed();
      }

      void reset(T* ptr) {
        decrement_ref_count_and_delete_if_needed();
        ptr_ = ptr;
        if (!ref_count_) {
          ref_count_ = new int{1};
        }
        *ref_count_ = 1;
      }

      int use_count() const noexcept {
        return *ref_count_;
      }

      void swap (shared_ptr& other) {
        std::swap(ptr_, other.ptr_);
        std::swap(ref_count_, other.ref_count_);
      }

      T& operator * () const {
        return *ptr_;
      }

      T* operator -> () const noexcept {
        return ptr_;
      }

      explicit operator bool() const noexcept {
        return (ptr_ != nullptr);
      }

      friend bool operator == (const shared_ptr& lhs, const shared_ptr& rhs) noexcept {
        return (lhs.ptr_ == rhs.ptr_);
      }

      friend bool operator != (const shared_ptr& lhs, const shared_ptr& rhs) noexcept {
        return !(lhs == rhs);
      }
  }; // class shared_ptr

  template <typename T>
  void swap(shared_ptr<T>& lhs, shared_ptr<T>& rhs) {   // swap function in namespace to facilitate ADL
    lhs.swap(rhs);
  }
} // namespace kapil

#endif

It might be difficult to get great feedback on this shared_ptr just because it omits so many of the features of std::shared_ptr. I mean I'd be tempted to just list all the features it doesn't have and then fail to review the code at all:

Atomic/thread-safe accesses to the reference count.
Implicit conversion from shared_ptr<T> to shared_ptr<U> whenever T* is convertible to U*.
shared_ptr<void>.
Custom and type-erased deleters.
The "aliasing constructor."
weak_ptr.
Implicit conversion from unique_ptr.
make_shared.

~shared_ptr() noexcept {             // destructor

The comment is redundant. And so is the noexcept: in C++11-and-later, destructors are implicitly noexcept, and it's idiomatic not to write it explicitly.

Contrariwise, it is extremely important to provide a noexcept swap function! You're missing the noexcept keyword here:

void swap (shared_ptr& other) {

By the way, it's weird to put a space between the name of the function and its opening parenthesis. Prefer swap(shared_ptr& other).

You've got a typo in a comment: assignent. Look for typos elsewhere. Where there's one mistake, there's often more than one. (Also, eliminate that redundant comment.)

      --(*ref_count_);
        if (*ref_count_ == 0) {

Your accesses to *ref_count already aren't thread-safe; but FYI, here's where the race condition would sneak in if you just went and made ref_count a pointer to a std::atomic<int>. In a multithreaded environment, it might well be that *ref_count_ == 0, but that doesn't mean that you're the one whose decrement took it to 0.

constexpr shared_ptr() noexcept : ptr_{nullptr}, ref_count_{nullptr} {}

If you used C++11 non-static data member initializers (NSDMIs) for ptr_ and ref_count_, you could =default this constructor.

T *ptr_ = nullptr;
int *ref_count_ = nullptr;

constexpr shared_ptr() noexcept = default;

shared_ptr& operator = (shared_ptr&& other) noexcept {   // move assignment operator
    *this = other;
}

Why did you bother implementing this overload at all, if you're not going to make it more efficient? Either omit it entirely (so that there's only one assignment operator), or implement it as e.g.

shared_ptr& operator=(shared_ptr&& other) noexcept {
    if (this != &other) {
        decrement_ref_count_and_delete_if_needed();
        ptr_ = std::exchange(other.ptr_, nullptr);
        ref_count_ = std::exchange(other.ref_count_, nullptr);
    }
    return *this;
}

By the way, once you make your swap function noexcept, you might consider saving some brain cells by using the copy-and-swap idiom:

shared_ptr& operator=(shared_ptr&& other) noexcept {
    shared_ptr(std::move(other)).swap(*this);
    return *this;
}

  void reset() noexcept {
    decrement_ref_count_and_delete_if_needed();
  }

This is wrong. Consider:

shared_ptr<int> p(new int);
shared_ptr<int> q = p;
p.reset();
assert(p.get() != nullptr);  // oops

  void reset(T* ptr) {
    decrement_ref_count_and_delete_if_needed();
    ptr_ = ptr;
    if (!ref_count_) {
      ref_count_ = new int{1};
    }
    *ref_count_ = 1;
  }

This is also wrong. Consider:

shared_ptr<int> p(new int);  // A
shared_ptr<int> q = p;       // B
p.reset(new int);            // C
p = shared_ptr<int>();       // D
assert(q.use_count() == 0);  // oops!

Line A sets the refcount to 1. Line B increments the refcount to 2. Line C repoints p.ptr_ (but does not change p.ref_count_ — oops!), and then resets the refcount to 1. Line D decrements the refcount to 0 and frees the int that was allocated on line C. Now q.ptr_ still points to the int that was allocated on line A, but *q.ref_count_ == 0.

The correct implementation of reset(T*) is simply

  void reset(T *ptr) {
      *this = shared_ptr(ptr);
  }

(I strongly advise against ever making calls to reset from user code, by the way, because every call to reset can be more readably expressed as a simple and type-safe assignment.)

int use_count() const noexcept {
    return *ref_count_;
}

There's an unspoken precondition here: that use_count() shall never be called on a shared_ptr in the default-constructed ("disengaged", "moved-from", "partially formed") state. If you keep this precondition, then the Lakos Rule would suggest that this function shouldn't be noexcept. However, I think it would be more natural to write

int use_count() const noexcept {
    return ref_count_ ? *ref_count_ : 0;
}

so that the function always has well-defined behavior.


Format raw bytes in hex and ascii in manner like Wireshark does This function formats raw buffer output in manner as Wireshark and many others does:

0x00: 68 65 6C 6C 6F 20 77 6F 72 6C 64 02 6B 68 67 61   |hello world.khga|
0x10: 76 73 64 20 0B 20 0A 05 58 61 73 6A 68 6C 61 73   |vsd . ..Xasjhlas|
0x20: 62 64 61 73 20 6A 61 6C 73 6A 64 6E 13 20 20 30   |bdas jalsjdn.  0|
0x30: 31 32 33 34 35 36 37 38 39                        |123456789       |

Please review and assess code quality.

std::string format_hex_payload(const char* payload, size_t payload_len)
{
    const char line_placeholder[] = "0x00:                                                   |                |";
    size_t number_of_lines = payload_len / 16 + (payload_len%16 > 0);
    size_t sizeof_output = sizeof line_placeholder * number_of_lines;
    char *output = (char*)alloca(sizeof_output);  //POSSIBLE REIMPL USE C++RVO std::string out; out.reserve(sizeof_output);

    char *pout = output;  //POSSIBLE REIMPL char *pout = out.data();
    const char *p = payload;
    const char *const end = payload + payload_len;
    size_t ascii_offset = strchr(line_placeholder,'|') - line_placeholder + 1;  //could be calculated at compile time

    unsigned short offset = 0;
    for(unsigned l=0; l < number_of_lines; l++, offset+=16)
    {
        char* pline_begin = pout;
        char* pline = pout;
        strcpy(pline,line_placeholder);
        pline += sprintf(pline, "0x%02X: ", offset);
        for(unsigned i=0; i<16 && p < end; ++i, ++p){
            pline += sprintf(pline, "%02X ", *p);
            *(pline_begin+ascii_offset+i) = isprint(*p) ? *p : '.';
        }
        *pline=' ';
        pout += sizeof line_placeholder;  // move pointer to next line
        pout[-1] = '\n';
    }
    pout[-1] = '\0';
    assert(pout == output + sizeof_output);  // sanity check

    return output;  //POSSIBLE REIMPL return out;
}

I think probably the first thing to do is decide whether you're really using C, or really using C++.

If you're really using C, then you need to get rid of the std::string, and allocate space for your return differently (e.g., using malloc).
If you're really using C++, then I'd at least consider using iostreams and manipulators to do most of the work. They are fairly verbose, but most people using C++ expect to deal with iostreams rather than raw buffers and C-style string manipulation.

I think I'd break the code up into a few more functions. For example, I'd probably have format_hex, which would repeatedly call format_line, which might in turn call some format_hex_value (or something on that order) to write out each individual value.
I'd probably avoid using the line placeholder, for a couple of reasons. The first (and probably most important) is that as you've done things right now, the last line is (as I see things) somewhat malformed. You've surrounded the ASCII part of the display with | as delimiters, but for that to make sense, you want the delimiter right next to the data, so that part of the last line should look like: |123456789|, so you immediately know there's no more data after the 9 (where your current display looks like there are space characters after the 9).
I'd definitely expand the addresses on the left out to at least 4 characters, and maybe more. In theory, you should probably be thinking in terms of 16 characters (allowing 64-bit addresses), but in all honesty I've rarely seen much point in hex dump of multiple gigabytes of data (but I certainly have seen good uses for more than 256 bytes).

Putting those together, one possibility (going the C++ route) might come out something on this order:

#include <cctype>
#include <iostream>
#include <iomanip>
#include <sstream>

struct hex {
    unsigned value;
    int digits;
public:
    hex(unsigned value, int digits) : value(value), digits(digits) { }

    friend std::ostream &operator<<(std::ostream &os, hex const &h) {
        auto oldfill = os.fill('0');

        // Yeah, iostreams get really verbose. Sorry.
        os << std::hex
           << std::setw(h.digits)
           << std::uppercase
           << std::setprecision(h.digits)
           << h.value;
        os.fill(oldfill);
        return os;
    }
};

void fmt_line(std::ostream &os, char const *data, size_t offset, size_t len) {
    os << hex(offset, 8) << ":";

    for (size_t i=0; i<len; i++) {
        os << " " << hex(data[offset+i], 2);
    }

    os << std::setw((16-len)*3 +2) << "|";
    for (size_t i=0; i<len; i++) {
        char ch = data[offset+i];
        os << (std::isprint((unsigned char)ch) ? ch : '.');
    }
    os << "|\n";
}

std::string format_hex(char const *data, size_t len) {
    unsigned lines = len/16;
    std::stringstream out;

    for (size_t line=0; line<lines; line++)
        fmt_line(out, data, line*16, 16);
    fmt_line(out, data, lines*16, len%16);
    return out.str();
}

int main() {
    char input[] = "hello world\02khgavsd \xb \xa\x5Xasjhlasbdas jalsjdn\xd  0123456789";
    std::cout << format_hex(input, sizeof(input));
}

This could use some further improvement. For one thing, it's probably not immediately obvious what some things like: os << std::setw((16-len)*3 +2) << "|"; are really doing, so they could undoubtedly use at least a little more work.


Binary search algorithm for non-overlapping time spans and possible gaps I have a data structure, containing time span nodes, with the following properties:

Nodes are sorted ascending
Time spans will not overlap, but may have gaps between them
Each node will have a start datetime and a finish datetime
The last node's finish may also be null (not finished yet)

I want to be able to find the node that intersects a given datetime, or return false if no match is found — basically resembling the following sql query for a sql table with similar properties as my data structure:

SELECT t.*
FROM table t
WHERE t.start <= @probeDate AND
    ( t.finish >= @probeDate OR
      t.finish IS NULL )

I've concluded that a (variation of a) binary search is probably the most efficient search algorithm. So, this is what I have come up with so far:

<?php
class Node
{
    protected $start;
    protected $finish;

    public function __construct( DateTime $start, DateTime $finish = null )
    {
        $this->start  = $start;
        $this->finish = $finish;
    }

    public function getStart()
    {
        return $this->start;
    }

    public function getFinish()
    {
        return $this->finish;
    }

    public function __toString()
    {
        /* for easy displaying of the timespan */
        return $this->start->format( 'H:i:s' ) . ' - ' . ( null !== $this->finish ? $this->finish->format( 'H:i:s' ) : '∞' );
    }
}

class NodeList
{
    protected $nodes;

    public function __construct( array $nodes = array() )
    {
        $this->nodes = $nodes;
    }

    public function find( DateTime $date )
    {
        $min = 0;
        $max = count( $this->nodes ) - 1;

        return $this->binarySearch( $date, $min, $max );
    }

    protected function binarySearch( $date, $min, $max )
    {
        if( $max < $min )
        {
            return false;
        }
        else
        {
            $mid = floor( $min + ( ( $max - $min ) / 2 ) );
            $node   = $this->nodes[ $mid ];
            $start  = $node->getStart();
            $finish = $node->getFinish();
            if( $date < $start )
            {
                return $this->binarySearch( $date, $min, $mid - 1 );
            }
            else if( $date > $finish )
            {
                if( $finish == null )
                {
                    return $node;
                }
                else
                {
                    return $this->binarySearch( $date, $mid + 1, $max );
                }
            }
            else
            {
                return $node;
            }
        }
    }
}

I am testing it with the following:

$nodes = array(
    new Node( new DateTime( '01:01:00' ), new DateTime( '01:05:00' ) ),
    new Node( new DateTime( '01:06:00' ), new DateTime( '01:10:00' ) ),
    new Node( new DateTime( '01:11:00' ), new DateTime( '01:15:00' ) ),
    new Node( new DateTime( '01:16:00' ), new DateTime( '01:20:00' ) ),
    new Node( new DateTime( '01:21:00' ), new DateTime( '01:25:00' ) ),
    new Node( new DateTime( '01:26:00' ), new DateTime( '01:30:00' ) ),
    new Node( new DateTime( '01:31:00' ), new DateTime( '01:35:00' ) ),
    new Node( new DateTime( '01:36:00' ), new DateTime( '01:40:00' ) ),
    new Node( new DateTime( '01:41:00' ) )
);

$list = new NodeList( $nodes );

$date = new DateTime( '01:00:00' );
for( $i = 0; $i < 100; $i++, $date->modify( '+30 seconds' ) )
{
    $node = $list->find( $date );
    echo 'find: ' . $date->format( 'H:i:s' ) . PHP_EOL;
    echo ( $node !== false ? (string) $node : 'false' ) . PHP_EOL . PHP_EOL;
}

Do you see any inherent flaws and/or ways to improve this algorithm? Perhaps you even know of a more time efficient algorithm to accomplish the same?

PS.: Don't worry about the internal integrity of NodeList and Node (checking for invalid nodes, invalid min and max datetimes, checking overlaps, etc.) — this is just a quick and dirty concept. I'm primarily interested in the search algorithm in NodeList::find() and consequently NodeList::binarySearch().

Flatter code is less arrow-shaped

In cases like this:

if( $max < $min )
{
    return false;
}
else
{
    // more deeply nested conditions ...
}

When the if branch ends with a return statement,
you can eliminate the else branch,
making the code flatter and less arrow-shaped,
which often makes it easier to read:

if( $max < $min )
{
    return false;
}

// more code ...

Fishy comparison

Although this is apparently valid PHP, comparing a date against a null value using the > operator seems like a dirty hack:

else if( $date > $finish )
{
    if( $finish == null )

I would reorganize the conditions in that piece of code:

if( $max < $min )
{
    return false;
}

$mid = floor( $min + ( ( $max - $min ) / 2 ) );
$node   = $this->nodes[ $mid ];
$start  = $node->getStart();
$finish = $node->getFinish();

if( $date < $start )
{
    return $this->binarySearch( $date, $min, $mid - 1 );
}

if( $finish == null )
{
    return $node;
}

if( $date > $finish )
{
    return $this->binarySearch( $date, $mid + 1, $max );
}

return $node;

Minor optimization

The find method recalculates the size of $this->nodes,
even though it cannot change after construction.
It would be better to calculate the size once,
at construction time.


Refactoring binary search algorithm in Ruby I wrote the binary search algorithm with Ruby, the code is below. The question is if there's any way to make it look cleaner?

def binary_search(sample_array, x, l, r)
  mid = (l + r)/2
  return -1 if r < l
  return binary_search(sample_array, x, l, mid-1) if (sample_array[mid] > x)
  return binary_search(sample_array, x, mid+1, r) if (sample_array[mid] < x)
  return mid if (sample_array[mid] == x)
end

result = binary_search(sample_array, x, l, r)
puts "#{result}"

It seems to me that returns can be minimised or even some more Ruby idioms added. Tried to use lambda, but smth went wrong.

Rather than defining it at the top level, why don't you make it a new method on Array?
Taking l and r makes recursive calls possible, but would be annoying for the end user who always has to pass 0 and array.length-1. You should either make those arguments optional, or don't use recursion, or use a private helper method for the recursion.
Someone suggested you could make it tail-recursive, but I don't think any Ruby implementation I know of can optimize tail recursion, so I'm not sure what the point of that would be. Method calls are expensive in Ruby, so if you are after performance, I think a while loop would be more appropriate.
You could consider calling <=> once and switching on the return value using case. I'm not sure if you would like the way this reads better or not. If performance is important, definitely try it and benchmark both ways.
If you do want to use if statements, the 3rd if is redundant.
You are using the type of binary search which has 3 different branches (depending on whether the value at the probed index is >, <, or == to the value you are searching for). There is another to write a binary search which only uses a single if-else, which is more concise, generally faster, and which is guaranteed to return the first matching element in the array (if there are duplicates).


Generate JSON File from a given YAML file I'm didn't use java for a long time(5 years ± ) and in the following code the  user should provide yaml file and it converting it to json file.

The code is working as expected but since I didn't use java for  a long time I want to see if I can write this code better and learn new things :-) in the latest java (currently with use of java8 not 9)

import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;

import java.io.BufferedWriter;
import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.util.Scanner;
import java.util.concurrent.atomic.AtomicReference;

public class yaml2json {

    public static void main(String[] args) {

        //Get File Path
        String sourceFilePath = getSourceFilePath(args);

        //Load File content
        AtomicReference<String> content = new AtomicReference<String>();
        try {
            content.set(readYamlFile(sourceFilePath));
        } catch (IOException e) {
            e.printStackTrace();
        }

        //Json string
        String jsonStr = null;
        try {
            jsonStr = convertYamlToJson(content.toString());
        } catch (IOException e) {
            e.printStackTrace();
        }

        //Print json file
        sourceFilePath = sourceFilePath.substring(0, sourceFilePath.length() - 3);
        sourceFilePath = sourceFilePath + "json";

        try {
            FileWriter fileWriter = new FileWriter(sourceFilePath);
            BufferedWriter bufferedWriter = new BufferedWriter(fileWriter);
            bufferedWriter.write(jsonStr);
            bufferedWriter.close();
            System.out.println("Success " + sourceFilePath);
        } catch (IOException err) {
            System.out.println("Error writing to file: " + err );
        }

    }

     private static String readYamlFile(String pathname) throws IOException {

        File file = new File(pathname);
        StringBuilder fileContents = new StringBuilder((int)file.length());
        Scanner scanner = new Scanner(file);
        String lineSeparator = System.getProperty("line.separator");

        try {
            while(scanner.hasNextLine()) {
                fileContents.append(scanner.nextLine() + lineSeparator);
            }
            return fileContents.toString();
        } finally {
            scanner.close();
        }
    }

    private static String getSourceFilePath(String[] args) {
        String inFile;
        if (args.length != 0) {
            inFile   = args[0];
        }else {
            inFile = "/Users/rayn/IdeaProjects/myproj/a.yml";

        }
        return inFile;
    }

    private static String convertYamlToJson(String yaml) throws IOException {

        ObjectMapper yamlReader = new ObjectMapper(new YAMLFactory());
        Object obj = yamlReader.readValue(yaml, Object.class);

        ObjectMapper jsonWriter = new ObjectMapper();
        return jsonWriter.writeValueAsString(obj);
    }

}

Simplify File Reading

Java 7 and 8 make I/O a lot simpler. You can simplify your reading of a String from your input file to this:

    private static String readYamlFile(String pathname) throws IOException {
        String lineSeparator = System.getProperty("line.separator");
        List<String> lines = Files.readAllLines(Paths.get(pathname));

        return lines.stream().collect(Collectors.joining(lineSeparator));
    }

Remove the first two Try...Catch block

Your first try...catch blocks catch IOException and then just print out the stack trace. This adds lots of uneccesarry lines for little benefit. If you add throws IOException to public static void main(String args[]) the code becomes clearer, such as:

 public static void main(String[] args) throws IOException {

    //Get File Path
    String sourceFilePath = getSourceFilePath(args);

    //Load File content
    AtomicReference<String> content = new AtomicReference<String>();
    content.set(readYamlFile(sourceFilePath));
    //... and so on

Remove the AtomicReference

The AtomicReference might be useful if the code were multi-threaded. But it's not, the content variable is local to main(). So it just needs to be a String and with the removal of the try..catch above it can be simplified to:

String content = readYamlFile(sourceFilePath);

Don't Use sourceFilePath for your outputFilePath

It's a misleading name, give the output file its own variable. You can also just do it in a single line:

String outputFilePath = sourceFilePath.substring(0, sourceFilePath.length() - 3) +
"json";

Also - this assumes the extension is three characters, so won't work correctly with a .yaml file. You can use a library like apache-commons to do this in a more foolproof way.

String outputFilePath = FilenameUtils.removeExtension(sourceFilePath)
         + ".json";

Simplify File Writing With PrintWriter

Again later versions of Java make dealing with I/O more straight-forward. I've kept the try...catch block so you can dispay the error message. You can also use the try-with resource to auto-close your PrintWriter if anything goes wrong

    try (PrintWriter out = new PrintWriter(outputFilePath)) {
        out.print(jsonStr);
        System.out.println("Success " + sourceFilePath);
    } catch (IOException err) {
        System.out.println("Error writing to file: " + err );
    }


Partial page refresh Ajax call on button click function RequestValidation() {
    $(".RequestTrigger").on("click", function () {
        var selectedButton = $(this);
        var arr = [];

        if ($(this).data('multiple') == true) {

            var ids = $('input:checkbox:checked.check-box').map(function () {
                var value = $(this).data('entityid');

                if (!isNaN(value)) {
                    arr.push(value);
                }
            });
        } else {
            var value = selectedButton.data('entityid');

            if (!isNaN(value)) {
                arr.push(value);
            }
        }

        var data = {
            ids: arr
        };

        $.ajax({
            url: selectedButton.data('url'),
            data: data,
            type: 'post',
            success: function (result) {
                $('#largeModal').modal('hide');
                $('#ApprovalTable').html(result);
                checkBox();
                accountRequestValidation();
            }

        });

    });

};

This functions binds to buttons with .RequestTrigger If the button has a data-attribute called multiple with a value of true the functions gets the entity-id of every checkbox and passes the values to the ajax request or if the value of multiple is false then it just gets the entity-id of the button clicked

I would suggest to:

I would recommend to use camelCase naming style, so instead of ApprovalTable you could use approvalTable
you are using a function to define a handler for an event, the name is not very consistent with what this function does, you could rename it to enableRequestValidationHandler
It's often a good idea to cache selectors, if you plan to use them more than once, so instead of using directly $('#largeModal') you could use somewhere:

var largeModal = $('#largeModal');

then use it in the ajax response handler via variable success: function (result) {...

The reason to do that, is because jQuery dive in html everytime you ask him to find something in html

you already defined var selectedButton = $(this); but still use $(this) in your code, you could use instead the selectedButton variable. I usually call it: self

These lessons will help you to understand fast and easy how jquery works


Find meaningful words from the input string Given a string find the number of meaningful words which could be formed from the string
for eg. programmerit forms:

pro+gram+merit
program+merit
programmer+it
pro+grammer+it

Looking for code review optimizations and best practices. Also hoping for suggestions for complexity better than \$O(n^2)\$, where n is the number of characters.

public final class DictionaryValidWords {

    private static final Set<String> dictionary = new TreeSet<String>();
    static {
        dictionary.add("this");
        dictionary.add("his");
        dictionary.add("is");
        dictionary.add("awe");
        dictionary.add("we");
        dictionary.add("some");
        dictionary.add("awesome");
        dictionary.add("foo");
        dictionary.add("bar");
    }

    private DictionaryValidWords() {}

    /**
     * Returns set of valid words given an input string.
     * It eliminates duplicates.
     *
     * @param str   The input string whose valid words need to be found out.
     * @return      List of valid words nested in the string.
     */
    public static Set<String> findValidStrings(String str) {
        if (str.length() ==  0) {
            throw new IllegalArgumentException("Strings of length 0 are illegal");
        }

        final Set<String> validWords = new HashSet<String>();
        for (int i = 0; i < str.length(); i++) {
            StringBuilder sb = new StringBuilder();
            for (int j = i; j < str.length(); j++) {
                sb.append(str.charAt(j)); // O(1) complexity.
                if (dictionary.contains(sb.toString())) {
                    validWords.add(sb.toString());
                }
            }
        }
        return validWords;
    }
}

public class DictionartValidWordTest {

    @Test
    public void testValidWords() {
        Set<String> expectedSet = new HashSet<String>(Arrays.asList("awe", "is", "his", "awesome", "some", "this", "we"));
        assertEquals(expectedSet, DictionaryValidWords.findValidStrings("thisisawesome"));
    }
}

I only have one minor objection for the moment...
...
...
Why are you not using dependency injection??
OK, it turned out to be a major one. (I did consider using caps-lock!)
Consider a situation where you would want to do this operation for English words, and then for French words. Instead of using all things static, make a public constructor
private final Set<String> dictionary;

public DictionaryValidWords(Set<String> dictionary) {
    this.dictionary = dictionary;
}

You might consider this as "outside the core part of the code" but it's really not. And it is a very important change to make, class-design wise and best-practices wise.
You could then use a DictionaryFactory class where you could have methods (static or non-static, as you prefer), for exampleDictionary (your current dictionary), english, french, etc...


Norms, rules or guidelines for calculating and showing ETA/ETC for a process ETC = "Estimated Time of Completion"

I'm counting the time it takes to run through a loop and showing the user some numbers that tells him/her how much time, approximately, the full process will take. I feel like this is a common thing that everyone does on occasion and I would like to know if you have any guidelines that you follow.

Here's an example I'm using at the moment:

int itemsLeft; //This holds the number of items to run through.
double timeLeft;
TimeSpan TsTimeLeft;
list<double> avrage;
double milliseconds; //This holds the time each loop takes to complete, reset every loop.

//The background worker calls this event once for each item. The total number
//of items are in the hundreds for this particular application and every loop takes
//roughly one second.
private void backgroundWorker1_ProgressChanged(object sender, ProgressChangedEventArgs e)
{
    //An item has been completed!

    itemsLeft--;
    avrage.Add(milliseconds);

    //Get an avgrage time per item and multiply it with items left.
    timeLeft = avrage.Sum() / avrage.Count * itemsLeft;
    TsTimeLeft = TimeSpan.FromSeconds(timeLeft);

    this.Text = String.Format("ETC: {0}:{1:D2}:{2:D2} ({3:N2}s/file)",
        TsTimeLeft.Hours,
        TsTimeLeft.Minutes,
        TsTimeLeft.Seconds,
        avrage.Sum() / avrage.Count);

    //Only using the last 20-30 logs in the calculation to prevent an unnecessarily long List<>.
    if (avrage.Count > 30)
        avrage.RemoveRange(0, 10);

    milliseconds = 0;
}

//this.profiler.Interval = 10;
private void profiler_Tick(object sender, EventArgs e)
{
    milliseconds += 0.01;
}

As I am a programmer at the very start of my career I'm curious to see what you would do in this situation. My main concern is the fact that I calculate and update the UI for every loop, is this bad practice?

Are there any do's/don't's when it comes to estimations like this? Are there any preferred ways of doing it, e.g. update every second, update every ten logs, calculate and update UI separately? Also when would an ETA/ETC be a good/bad idea.

Your approach looks reasonable if one makes the assumption that the average time for item processing is roughly the same for each item.

Updating the UI once a second is ok, updating it more often does not make much sense since the typically user cannot follow that visually either. If this really becomes a bottleneck, you might consider to add some code like

if(timeWhenUIWasUpdated < currentTime-1.0)
{
    this.Text = String.Format(/*...*/);
    timeWhenUIWasUpdated = currentTime; // unit: seconds
}

(you will have to introduce those two variables timeWhenUIWasUpdated and currentTime into your code).

Of course, when your progress event gets called more often (for example, several 10,000s a second), you will have to think about further optimizations, since even without the UI you will have to do some time value management or calculations. But as long as this is not the case, keep things simple.

This one here

if (avrage.Count > 30)
     avrage.RemoveRange(0, 10);

looks like premature optimization, I guess

     avrage.RemoveRange(0, 1);

will give you a more smooth behaviour (and I guess you won't notice the difference in running time in most real-world cases).

Furthermore, for testing purposes, you may consider to refactor the average-time calculation into a SlidingAverageCalculator class, separating the logic fully from the background worker and the UI display. This will make it really easy to write unit tests for that part of the code.