Get Worksheet Data Array (Standard Methods) I'm re-writing my module of Standard Methods. Virtually every project I do begins with grabbing some number of Data Tables and putting them in arrays. So, this is my general "Get Worksheet Data" method(s).

As always, particularly interested in maintainability, but all feedback is welcome.

Public Function GetWsDataArray(ByRef wbTarget As Workbook, ByRef wsTarget As Worksheet, ByVal topLeftCellText As String, ByVal useCurrentRegion As Boolean _
                            , Optional ByVal searchStartRow As Long = 1, Optional ByVal searchStartColumn As Long = 1 _
                            , Optional ByVal searchEndRow As Long = 10, Optional ByVal searchEndColumn As Long = 10) As Variant
                            '/ 10x10 is arbitrary search range that should cover almost all typical worksheets

    Dim dataArray As Variant
        dataArray = Array()
        dataArray = GetWsDataRange(wbTarget, wsTarget, topLeftCellText, useCurrentRegion, searchStartRow, searchStartColumn, searchEndRow, searchEndColumn)

        GetWsDataArray = dataArray

End Function

Public Function GetWsDataRange(ByRef wbTarget As Workbook, ByRef wsTarget As Worksheet, ByVal topLeftCellText As String, ByVal useCurrentRegion As Boolean _
                            , ByVal searchStartRow As Long, ByVal searchStartColumn As Long _
                            , ByVal searchEndRow As Long, ByVal searchEndColumn As Long) As Range

    Dim wbSource As Workbook, wsSource As Worksheet
    Set wbSource = ActiveWorkbook
    Set wsSource = ActiveSheet

    wbTarget.Activate
    wsTarget.Activate
    UnhideWsCellsAndRemoveFilters wsTarget

    Dim topLeftCell As Range, searchRange As Range, dataRange As Range
    Set searchRange = wsTarget.Range(Cells(searchStartRow, searchStartColumn), Cells(searchEndRow, searchEndColumn))
    Set topLeftCell = CellContainingStringInRange(searchRange, topLeftCellText)

    Dim lastRow As Long, lastCol As Long
    If useCurrentRegion Then
        Set dataRange = topLeftCell.CurrentRegion
    Else
        lastRow = Cells(Rows.Count, topLeftCell.Column).End(xlUp).row
        lastCol = Cells(topLeftCell.row, Columns.Count).End(xlToLeft).Column
        Set dataRange = wsTarget.Range(topLeftCell, Cells(lastRow, lastCol))
    End If

    Set GetWsDataRange = dataRange
    wbSource.Activate
    wsSource.Activate

End Function

Public Function CellContainingStringInRange(ByRef rngSearch As Range, ByVal strSearch As String) As Range

    Dim errorMessage As String

    Set CellContainingStringInRange = rngSearch.Find(strSearch, LookIn:=xlValues)
    If CellContainingStringInRange Is Nothing _
        Then
            errorMessage = "Couldn't find cell """ & strSearch & """ in " & rngSearch.Worksheet.name
            PrintErrorMessage errorMessage, stopExecution:=True
    End If

End Function

Public Sub UnhideWsCellsAndRemoveFilters(ByRef ws As Worksheet)

    ws.Rows.Hidden = False
    ws.Columns.Hidden = False
    ws.AutoFilterMode = False

End Sub

UnhideWsCellsAndRemoveFilters()

Well thats a real strange name. Wouldn't it be better to replace Unhide with Show? Like ShowWsCellsAndRemoveFilters() ?

CellContainingStringInRange()

You should declare your variables as near as possible to their usage. This together with reverting the if condition to return early would lead to

Public Function CellContainingStringInRange(ByRef rngSearch As Range, ByVal strSearch As String) As Range

    Set CellContainingStringInRange = rngSearch.Find(strSearch, LookIn:=xlValues)
    If CellContainingStringInRange IsNot Nothing Then
        Exit Function
    End If

    Dim errorMessage As String
    errorMessage = "Couldn't find cell """ & strSearch & """ in " & rngSearch.Worksheet.name
    PrintErrorMessage errorMessage, stopExecution:=True
End Function

GetWsDataRange()

At first glance at this

Dim wbSource As Workbook, wsSource As Worksheet
Set wbSource = ActiveWorkbook
Set wsSource = ActiveSheet

it will be overseen that wbSource and wsSource are different objects. You should consider to use some vertical spacing to separate these and/or to use different names like sourceBook and sourceSheet. The same is true for the method arguments wbTarget and wsTarget.

Generally declaring multiple variables on the same line should be avoided because it is much harder to read/maintain.

Having the dataRange As Range doesn't provide any value because it is only used "once". You can assign the return value directly.

Implementing this points and having the declarations as near to their usage as possible and on separate lines will lead to

Public Function GetWsDataRange(ByRef targetBook As Workbook, ByRef targetSheet As Worksheet, ByVal topLeftCellText As String, ByVal useCurrentRegion As Boolean _
                            , ByVal searchStartRow As Long, ByVal searchStartColumn As Long _
                            , ByVal searchEndRow As Long, ByVal searchEndColumn As Long) As Range

    Dim sourceBook As Workbook
    Set sourceBook = ActiveWorkbook

    Dim sourceSheet As Worksheet
    Set sourceSheet = ActiveSheet

    targetBook.Activate
    targetSheet.Activate
    ShowWsCellsAndRemoveFilters ws

    Dim searchRange As Range
    Set searchRange = targetSheet.Range(Cells(searchStartRow, searchStartColumn), Cells(searchEndRow, searchEndColumn))

    Dim topLeftCell As Range
    Set topLeftCell = CellContainingStringInRange(searchRange, topLeftCellText)

    Dim dataRange As Range

    If useCurrentRegion Then
        Set GetWsDataRange = topLeftCell.CurrentRegion
    Else
        Dim lastRow As Long
        lastRow = Cells(Rows.Count, topLeftCell.Column).End(xlUp).row

        Dim lastCol As Long
        lastCol = Cells(topLeftCell.row, Columns.Count).End(xlToLeft).Column

        Set GetWsDataRange = targetSheet.Range(topLeftCell, Cells(lastRow, lastCol))
    End If

    sourceBook.Activate
    sourceSheet.Activate

End Function


Queue reconstruction by height This is the "Queue Reconstruction by Height" problem from leetcode.com:

Suppose you have a random list of people standing in a queue. Each person is described by a pair of integers (h, k), where h is the height of the person and k is the number of people in front of this person who have a height greater than or equal to h. Write an algorithm to reconstruct the queue.
Note: The number of people is less than 1100.

Suppose I take only the tallest persons, all having the same maximum height. Their second values must be 0, 1, 2, 3... with no gaps at all, because they only count each other. Therefore, if there were no other persons at all, their second value must be their final index. What about the persons with second maximum height then? Suppose there are only tallest persons and just one more person who has slightly smaller height. What would be his position? Well, since he obviously only count tallest persons, his position would still be his second value. The next person of the same height counts only the previous person and all the tallest ones, but since they are all already in the queue, his second value would also be his index.
import collections

class Solution(object):
    def reconstructQueue(self, people):
        """
        :type people: List[List[int]]
        :rtype: List[List[int]]
        """
        d = collections.defaultdict(list)
        for height, count in people:
            d[height] += [count]

        recon_q = []
        for height, counts in sorted(d.items(), key = lambda x:x[0], reverse=True):
            for count in sorted(counts):
                recon_q.insert(count, [height, count])
        return recon_q

Is there any better data structure that can be used? I feel there must be a better way to do sorting here.
Note that the Solution class is required by LeetCode, even though it has no other function.

There are two difficulties here:

The input needs to be sorted in decreasing order by height, but increasing order by count. The code in the post handles this by collating the input by height and then sorting twice: first in reverse order by height, and then for a given height in normal order by count.

This could be simplified by passing a key function to sorted so that it sorts in the required order. In particular, if take the key to be (-height, count) then the list will sort into the required order without any need for collation.

def _queue_key(person):
    height, count = person
    return -height, count

def reconstruct_queue_2(people):
    result = []
    for person in sorted(people, key=_queue_key):
        _, count = person
        result.insert(count, person)
    return result

Inserting an item into the middle a list takes time proportional to the length of the list (see the TimeComplexity page in the Python wiki). This means that the reconstruction algorithm takes time that's quadratic in the length of the queue. It would nice to do better than this.

The data structure that's needed here is some kind of balanced tree. Python doesn't have anything suitable in the standard library, but the sortedcontainers package from the Python Package Index has a SortedListWithKey class that can do what we need.

from sortedcontainers import SortedListWithKey

def reconstruct_queue_3(people):
    result = SortedListWithKey(key=lambda _:0)
    for person in sorted(people, key=_queue_key):
        _, count = person
        result.insert(count, person)
    return result

This is slightly tricksy code because we're not making use of the "sorted" property of the SortedListWithKey class — we use the key= argument to make all the items identical as far as sort order is concerned. We're using the class only to get efficient insertion at an arbitrary index in a list.


Booking system for shows Kindly accept my apologies as Java is something my mind cannot digest no matter what I do. At the moment I have an assignment and have written a piece of code and have used Constructor (I think so) but not sure if I have used it correctly and if Yes then will need some guidance in order to refactor the code.

Anyhow I am writing code for a Booking System where user is given option to input value and create a new Show along with date. I have achieved this in the below code but note sure that I have used the constructor properly or not.

import java.io.*;
import java.util.*;

public class LocalPlay extends Play {

    private static String playName;
    private static String playDate;

    private LocalPlay(String playName, String playDate) {
        this.playName = playName;
        this.playDate = playDate;
    }

    public String getShowName() {
        return playName;
    }

    public String getShowDate() {
        return playDate;
    }

    public void setShowName(String value) {
        this.playName = value;
    }

    public void setShowDate(String date) {
        this.playDate = date;
    }

    public static void lPlayDetails() {
        try {
            int selection = 0;
            Scanner option = new Scanner(System.in);
            System.out.println();
            System.out.println("Press 1 to Add a New Play/ Show: ");
            System.out.println("Press 2 to View existing Play: ");
            System.out.println("Press 3 to delete existing Play: ");
            System.out.println("Press 4 for Previous Menu: ");
            System.out.println();
            System.out.print("Kindly make the selection: ");
            selection = option.nextInt();
            if (selection == 1) {
                addPlay();
            } else if (selection == 2) {
                viewPlay();
            } else if (selection == 3) {
                deletePlay();
            } else if (selection == 4) {
                managePlay();
            } else
                while (selection != 1 || selection != 2 || selection != 3) {
                    System.out.println();
                    System.out.println(selection + " is not a valid option. ");
                    managePlay();

                }
        } catch (InputMismatchException ime) {
            System.out.println("Invalid option selected.");
            System.out.println("Try again");
            lPlayDetails();
        }
    }

    private static void addPlay() {
        String name;
        String date;
        Scanner option = new Scanner(System.in);
        System.out.print("Enter name of Play: ");
        name = option.nextLine();
        System.out.print("Enter date of Play: ");
        date = option.nextLine();
        LocalPlay lp = new LocalPlay(name, date);
        System.out.println("The name of play is " + playDate + " date played is " + playDate);
        lPlayDetails();
        BufferedWriter writer = null;
        try {
            writer = new BufferedWriter(new FileWriter("play.txt"));
            writer.write(name + " " + date);

        } catch (IOException e) {
        } finally {
            try {
                if (writer != null)
                    writer.close();
            } catch (IOException e) {
            }
        }
        return;
    }

    private static void deletePlay() {
        System.out.println("Method not implemented");
        lPlayDetails();

    }

    private static void viewPlay() {
        System.out.println("Method not implemented");
        lPlayDetails();

    }

}

Structure

Generally, a method should do one thing. For example addPlay should add a play (although it's an odd method for a play class, as a play generally cannot own another play). But your method doesn't just add something, it actually retrieves user input, prints stuff, writes stuff to a file, and then calls a seemingly unrelated method, which asks for more user input and processes it in some way.

It's also very unclear what your LocalPlay class is actually supposed to do. Looking at the fields, constructor, and getter/setter, it looks as if it holds the data for one play, but then it is also possible to add a play, which doesn't make sense.

You should definitely try to separate the holding of the data from the parsing of user input.

You should also try to better define the tasks and differences of lPlayDetails and addPlay, and you should try to remove the confusing call structure (the functions call each other, which makes it difficult to follow the control flow of your program).

Misc

there is no reason to make the fields (or the methods) static.
your naming is a bit confusing. There doesn't seem to be a difference between a play and a show, you seem to be using both interchangeably. In that case, use just one of the terms for clarity.
if a method is not implemented, it shouldn't do anything. Instead of doing something totally different, throw something like an UnsupportedOperationException.
lPlayDetails: It's very unclear what this method does. The name is vague and it is missing a JavaDoc comment. It is also very unclear what the exact difference to addPlay is.
it is very unclear to me why you create a new LocalPlay in addPlay. There doesn't seem to be a reason for it, and it seems to be unused.


ChangeCalculator for calculating money denominations in change I'm doing some exercises in OOP and I just created a program that calculates amounts of denominations in a change at the shop. So, let's say something costs 10, I give 25.30 (for some reason), so I should get as a change: one 10, one 5, one 0.20, one 0.10.

When I started to create the program, I thought it would be nice if client of the ChangeCalculator had an option to specify his way of calculating the amounts - for example to use his own currency. That's why I created interfaces.

My program consists of:

IChangeCalculator - represents change calculator
ChangeCalculatorPLN - implementation that returns MoneyAmountPLN (money denominations amounts in PLN currency)
IMoneyAmount - represents data about amounts of denominations
MoneyAmountPLN - represents money denominations amounts in PLN
IMoneyToCoinsConverter - represents converter that converts given money to something that implements IMoneyAmount
MoneyToCoinsConverterPLN - converts given amount of money to MoneyAmountPLN

Here is the source code:

public interface IChangeCalculator<T> where T : IMoneyAmount
{
    T Calculate(decimal price, decimal payedAmount);
}

public class ChangeCalculatorPLN : IChangeCalculator<MoneyAmountPLN>
{
    IMoneyToCoinsConverter<MoneyAmountPLN> _moneyToCoinsConverter;

    public ChangeCalculatorPLN( IMoneyToCoinsConverter<MoneyAmountPLN> moneyToCoinsConverter )
    {
        _moneyToCoinsConverter = moneyToCoinsConverter;
    }

    public MoneyAmountPLN Calculate(decimal price, decimal payedAmount)
    {
        var change = payedAmount - price;
        if (change < 0)
            throw new Exception("The given amount of money is not enough!");
        if (change == 0)
            return MoneyAmountPLN.Empty;

        var amount = GetAmount(change);
        return amount;
    }

    private MoneyAmountPLN GetAmount(decimal change)
    {
        return _moneyToCoinsConverter.Convert(change);
    }
}

public interface IMoneyAmount
{
    IEnumerable<int> GetAmounts( );
}

//MoneyAmountPLN is quite long, because it contains fields for each denomination: 500, 200, 100, 50, 20, 10, 1, 0.5, 0.2, 0.1, 0.05, 0.02, 0.01. I'll not put it here.

public interface IMoneyToCoinsConverter<T> where T : IMoneyAmount
{
    T Convert(decimal money);
}

public class MoneyToCoinsConverterPLN : IMoneyToCoinsConverter<MoneyAmountPLN>
{
    public MoneyAmountPLN Convert(decimal money)
    {
        //logic doesn't really matter here. It returns MoneyAmountPLN with corrects amounts of denominations.
    }

}

The program works, I did some unit tests. My real issue is that I'm not sure if this implementation is a good one. Did I do some mistakes? Could some parts be done better? Personally I think that maybe the structure is a little too complicated? If I saw a program like this for the first time, I would probably feel lost, but I'm not a very good programmer, so I can't say if my feeling is right.
Adding new currency requires creation of new classes that implement: IChangeCalculator, IMoneyAmount, IMoneyToCoinsConverter.
I think that this is too much. I think that ChangeCalculator shouldn't need to be reimplemented for each new currency, but I didn't know how to do that, since each currency requires different IMoneyAmount implementation. Calculate method of ChangeCalculator needs to know what type of IMoneyAmount it returns.

Please provide constructive feedback, it would be great to hear any of your advices.

The main problem with your code is that you've been encoding currency denomination data in C#'s type system: one type for each currency. That somehow led to a complicated interface/generics design, all without providing any real benefits. In fact, this design has several drawbacks:

Each new currency requires additional code (3 extra classes in total).
Apparently the core money-to-coins algorithm needs to be duplicated for each new currency - a proper design should make this reusable across different currencies.
Those generic interfaces aren't very useful: IChangeCalculator<MoneyAmountPLN> is not interchangeable with IChangeCalculator<MoneyAmountEuro>, despite being the 'same' interface.
The IChangeCalculator layer doesn't seem to provide much value: this could easily be done at the call-site.
Wrapping the results in an IMoneyAmount object on which the caller has to call a GetAmounts method seems a little pointless - why not just return those amounts directly?

This can be simplified a lot with a data-driven approach. The core money-to-coins method needs an amount and knowledge about a currency's denominations, and returns a list of coin (or bill) amounts:

public (int amount, decimal denomination)[] SplitIntoDenominations(
    decimal amount,
    decimal[] denominations)

Such a method can easily be reused for different currencies:

var euroDenominations = new decimal[] { 500m, 200m, 100m, ..., 0.05m, 0.02m, 0.01m };
// Denomination arrays can be stored anywhere: in hard-coded arrays,
// in a currency-name/denominations dictionary, or loaded from a file, etc.

var returned = SplitIntoDenominations(amount, euroDenominations);
foreach ((var amount, var denomination) in returned)
    Console.WriteLine($"{amount} coins of €{denomination}.");

You may find it useful to create a CurrencyInfo class that contains a denominations array as well as the name and symbol of the currency, and perhaps additional information, depending on your needs:

var euro = new CurrencyInfo("Euro", "€", new decimal[] { 500m, ..., 0.01m });
var dollar = new CurrencyInfo("Dollar", "$", new decimal[] { 100m, ..., 0.01m });

The same can be done for the return value of this method, if you find an array of named tuples to be insufficient. Again, this depends on your needs, but keep in mind that you only need a single class, regardless of currency.


RSpec tests for an invoice mailer How would you DRY up this RSpec code?

require "spec_helper"

describe InvoiceMailer do
  let(:user) { create(:user) }
  let(:student) { create(:student, user: user) }
  let(:body) { "And I for one will join with anyone, don't care what color you are. As long as you want change this miserable condition that exists on this earth." }
  subject(:mail) { InvoiceMailer.issue(student, body) }

  describe "issue" do
    subject(:mail) { InvoiceMailer.issue(student, body) }

    it "renders the subject" do
      expect(mail.subject).to eq "New Invoice!"
    end

    it "renders the receiver email" do
      expect(mail.to).to eq [student.user.email]
    end

    it "renders the sender email" do
      expect(mail.from).to eq ["info@test.com"]
    end
  end

  describe "rescind" do
    subject(:mail) { InvoiceMailer.rescind(student, body) }

    it "renders the subject" do
      expect(mail.subject).to eq "Invoice Rescinded!"
    end

    it "renders the receiver email" do
      expect(mail.to).to eq [student.user.email]
    end

    it "renders the sender email" do
      expect(mail.from).to eq ["info@test.com"]
    end
  end

end

You can use the its syntax:

require "spec_helper"

describe InvoiceMailer do
  let(:user) { create(:user) }
  let(:student) { create(:student, user: user) }
  let(:body) { "And I for one will join with anyone, don't care what color you are. As long as you want change this miserable condition that exists on this earth." }

  describe "issue" do
    subject { InvoiceMailer.issue(student, body) }

    its(:subject) { should eq "New Invoice!" }

    its(:to) { should eq [student.user.email] }

    its(:from) { should eq ["info@test.com"] }
  end

  describe "rescind" do
    subject { InvoiceMailer.rescind(student, body) }

    its(:subject) { should eq "Invoice Rescinded!" }

    its(:to) { should eq [student.user.email] }

    its(:from) { should eq ["info@test.com"] }
  end

end

You can further DRY it up using example groups

describe InvoiceMailer do
  let(:user) { create(:user) }
  let(:student) { create(:student, user: user) }
  let(:body) { "And I for one will join with anyone, don't care what color you are. As long as you want change this miserable condition that exists on this earth." }

  shared_examples_for "outgoing mail" do |method, mail_subject|
    subject { InvoiceMailer.send(method, student, body) }

    its(:subject) { should eq mail_subject }

    its(:to) { should eq [student.user.email] }

    its(:from) { should eq ["info@test.com"] }
  end

  describe "issue" do
    it_should_behave_like "outgoing mail", :issue, 'New Invoice!'
  end

  describe "rescind" do
    it_should_behave_like "outgoing mail", :rescind, 'Invoice Rescinded!'
  end
end


Finding the longest common substring between two tweets I retrieved 200 tweets using the Jersey API. I want to find two tweets which have the longest common substring.

tweetList is an ArrayList of Tweet objects. The comapreTweets method compares tweet objects for longest substring.

Tweet t1=new Tweet();Tweet t2=new Tweet();
int longestString=0;

for(int i=0;i<tweetList.size();i++)
{int store=0;Tweet comparer=null;
    for(int j=i+1;j<tweetList.size();j++)
    {
        if(j!=i){
        int result=tweetList.get(i).compareTweets(tweetList.get(j));
        if(result>store){
            store=result;
            comparer=tweetList.get(j);
        }}
    }
    if(longestString<store)
    {
        longestString=store;
        t1=tweetList.get(i); t2=comparer;
    }
}

If I retrieve 200 tweets then this will loop approx. 40000 times. I require a more efficient way.

The classic solution to this well-known problem is using a Suffix array (see also Suffix tree and Generalized Suffix tree which is a more general data structure for these kinds of problems.). Basically, a Suffix array is a sorted set in which every suffix of every tweet is inserted. By using the prefixes of those sorted suffixes, you can find the longest common substring by iterating through every pair of substrings from different tweets in the Suffix array.

Construction of all suffixes can be performed in O(140 k) time, where k is the the amount of tweets, and 140 is the length of a tweet. These can then be sorted in O(k * log(k)) time. Finally allowing you to find the longest common substring in O(k 140) time. Which results in roughly 57 800 operations.

The algorithm proposed in your orignal post loops through every tweet twice, and has to, worst case, compare all the characters. This leaves a total complexity of O(k^2 * 140) which is 5 600 000 operations.

Example

Imagine you have three strings:

ababa
bataba
dafjkl

We generate all suffixes of every string. Every suffix of the first string is:

ababa
baba
aba
ba
a

For the second we have:

batabak
atabak
tabak
abak
bak
ak
a

And finally for the third:

dafjkl
afjkl
fjkl
jkl
kl
l

If we sort all suffixes for all strings, we get the following sorted set:

a
a
aba
ababa
abak
afjkl
ak
atabak
ba
baba
bak
batabak
dafjkl
fjkl
jkl
kl
l
tabak

Thus we iterate through every pair of suffixes for different strings, to find the longest common prefix in suffixes from two different strings. This is equal to the longest common substring.

First encounter is a (origining from the 1st string) and a (2nd string), thus our longest common substring (LCS) is of length 1 and equal to a.

We go to the next pair from different strings, which is aba/ababa and abak, both of length 3. This is set as the LCS so far. If you keep going through the set using this algorithm, you will find that this is the LCS for these three strings.

I do not know Java, however, I just created a C++ solution to a very similar problem. The solution for your problem is essentially the same, you just have to generate substrings for k strings rather than two.


HackerRank: Friend Circles (DFS in Java) I got this problem on a HackerRank challenge and this was the best I could come up with. It's basically just using DFS to find connected components (in this case, friend circles). I was having a tough time figuring out how to track unvisited nodes. Please let me know how I can improve this code. It appears to work, but the test cases given were quite simple.

Problem:

There are N students in a class. Some of them are friends, while some are not. Their friendship is transitive in nature, i.e., if A is friend of B and B is friend of C, then A is also friend of C. A friend circle is a group of students who are directly or indirectly friends.

You have to complete a function int friendCircles(char[][] friends) which returns the number of friend circles in the class. Its argument, friends, is a NxN matrix which consists of characters "Y" or "N". If friends[i][j] == "Y" then i-th and j-th students are friends with each other, otherwise not. You have to return the total number of friend circles in the class.

Sample Input 0:
4
YYNN
YYYN
NYYN
NNNY
Sample Output 0:
2

Explanation 0:
There are two pairs of friends [0, 1] and [1, 2]. So [0, 2] is also a pair of friends by transitivity.
So first friend circle contains (0, 1, 2) and second friend circle contains only student 3

Sample Input 1:
5
YNNNN
NYNNN
NNYNN
NNNYN
NNNNY
Sample output 1:
5

Constraints (sorry, couldn't get formatting down so I had to put the constraints down here):

1 <= N <= 300.
Each element of matrix friends will be "Y" or "N".
Number of rows and columns will be equal in friends.
friends[i][j] = "Y", where 0 <= i < N.
friends[i][j] = friends[j][i], where 0 <= i < j < N.

Solution:

import java.util.ArrayDeque;
import java.util.Deque;
import java.util.HashSet;
import java.util.Set;

public class Solution {
    public static int friendsCircle(char[][] friends) {
        // The only alternative I could think of, instead of
        // tracking unvisited nodes, was to put visited nodes
        // in a set and then do setOfAllNodes.removeAll(visited)
        // to see which nodes are still unvisited
        Set<Integer> unvisited = new HashSet<>();
        boolean[] visited = new boolean[friends.length];
        Deque<Integer> stack = new ArrayDeque<>();
        int connectedComponents = 0;
        for (int i = 0; i < friends.length; i++) {
            unvisited.add(i);
        }
        // dfs on friends matrix
        while (!unvisited.isEmpty()) {
            stack.push(unvisited.iterator().next());
            connectedComponents++;
            while (!stack.isEmpty()) {
                int currVertex = stack.pop();
                if (visited[currVertex] == false) {
                    visited[currVertex] = true;
                    unvisited.remove(currVertex);
                    for (int i = 0; i < friends[currVertex].length; i++) {
                        if (friends[currVertex][i] == 'Y' && visited[i] == false) {
                            stack.push(i);
                        }
                    }
                }
            }
        }
        return connectedComponents;
    }

    public static void main(String[] args) {
        char[][] friends = {
                {'Y','Y','N','N'},
                {'Y','Y','Y','N'},
                {'N','Y','Y','N'},
                {'N','N','N','Y'}
        };
        System.out.println(friendsCircle(friends));
    }
}

Welcome to Code Review. Your code is good and easy to read, you can iterate over only half of the matrix (in my case the elements of the matrix a[i,j] with i < j) because from the test matrices always results a[i, j] = a[j, i] so friendship in symmetric .
I'm using a TreeMap<Integer, Set<Integer>> to store initial situation to guarantee natural order of the keys so it will always the lower index the index where friends will be added : everybody is friend of himself (reflexive property) so map[i]={ i }.

int n = friends.length;
Map<Integer, Set<Integer>> map = new TreeMap<>();
for (int i = 0; i < n; ++i) {
    Set<Integer> set = new TreeSet<>();
    set.add(i);
    map.put(i, set);
}

Now I check all elements a[i, j] with i < j and see if i appears as key in the map: if yes I add j to map[j], otherwise it means that i appears in another set and I will add j to this set. At the end in any case I will remove the key j:

for (int i = 0; i < n; ++i) {
    for (int j = 0; j < n; ++j) {
        if (i < j && friends[i][j] == 'Y') {
            if (map.containsKey(i)) {
                map.get(i).add(j);
            } else {
                for (Integer key : map.keySet()) {
                    Set<Integer> set = map.get(key);
                    if (set.contains(i)) {
                        set.add(j);
                    }
                }
            }
            map.remove(j);
        }
    }
}

The number of circles will coincide with the number of keys present in the map at the end:

public static int CountFriendsCircles(char[][] friends) {
    int n = friends.length;
    Map<Integer, Set<Integer>> map = new TreeMap<>();
    for (int i = 0; i < n; ++i) {
        Set<Integer> set = new TreeSet<>();
        set.add(i);
        map.put(i, set);
    }

    for (int i = 0; i < n; ++i) {
        for (int j = 0; j < n; ++j) {
            if (i < j && friends[i][j] == 'Y') {
                if (map.containsKey(i)) {
                    map.get(i).add(j);
                } else {
                    for (Integer key : map.keySet()) {
                        Set<Integer> set = map.get(key);
                        if (set.contains(i)) {
                            set.add(j);
                        }
                    }
                }
                map.remove(j);
            }
        }
    }

    return map.size();
}


Employee name auto-complete on card scan Im currently running an employee name substitution on card scan.
Overall the code takes the entry trough the gun scan, "scan" trough the hidden reference sheet to find a match and then input the actual complete name.
I had to implement that feature to my code so that names are always written the same (for futur data treatment purpose).
If y'all have some tip, hint or other proposition to optimise the code.
One of my goal would be for it to auto-fill with any entry (right now mostly working with operator numbers) or to suggest entries based on what's written in real time in a scrolling menu (if it's not too much).
Here's my codes :
Sub Workbook_sheetChange(ByVal Sh As Object, ByVal Target As Range)
Application.EnableEvents = False
Application.ScreenUpdating = False
If Not Intersect(Target, Sh.Range("C5:C8")) Is Nothing Then 'If change in "no.employé" cells

Call Employe ' Call name standardization macro

End If
Application.ScreenUpdating = True
Application.EnableEvents = True
End Sub

Sub Employe()
' Standardization of the operator name macro
' The code automaticly enters the employee names by is number entry
' Therefore permits employee to scan their pass and get their full names automaticly

Dim ash As Worksheet
Set ash = ActiveSheet ' Identify active sheet
Dim positionInitCaseNom1enY As Long: positionInitCaseNom1enY = 5 'First "Name" cell row position
Dim positionCaseNomEnX As Long: positionCaseNomEnX = 3           '"Name" cells culumn position
Dim k As long: k = positionInitCaseNom1enY                    ' Counter incrément position
Dim no As Variant  ' Value of name cell (changes every loop)
Dim i As Integer   ' Row integer

    For i = 0 To 3
    k = k + i       ' Up a row (first 0)
    no = ash.Cells(k, positionCaseNomEnX).Value ' Gives "no" the current "Name" cell value

        If no <> "" Then   ' Vérify if theres something in "Name" cell
        Dim nos As String
        nos = CStr(no)     ' no to string (now nos)
        Dim valeurnorm As Range
        Set valeurnorm = Sheets("Liste Employé").Range("A2:A200").Find(nos)
        ' Looking for a match in name reference sheet

            If valeurnorm Is Nothing Then ' If no match found
                If MsgBox("Entrer un numéro d'employé valide" & vbNewLine & "Le numéro d'employé entrée est le suivant: " & nos, 5 + vbCritical, "Erreur") = vbRetry Then
                ' Msgbox that the employee entered a false number and to retry

                    ash.Cells(k, positionCaseNomEnX).Value = "" ' Clear old number
                    ash.Cells(k, positionCaseNomEnX).Select     ' Reselect cases

                End If
            Else ' If name is in the reference sheet

                ash.Cells(k, positionCaseNomEnX).Value = valeurnorm ' Change the value of the entry for the complete name
            End If

        End If
  Next
End Sub

Sorry for all of the typos, i need to translate everything and my written english is kinda s***.

positionInitCaseNom1enY and positionCaseNomEnX should be declared as Const or in an Enum because they are default values that never change.
Const positionInitCaseNom1enY As Long = 5
Const positionCaseNomEnX As Long = 3

Do to implicit conversion no does not need to be cast to a String.  The compiler automatically does it for you.
Dim nos As String
nos = CStr(no)     ' no to string (now nos)

Use dynamic ranges whenever possible.
Set valeurnorm = Sheets("Liste Employé").Range("A2:A200").Find(nos)

Assuming the name list is the only thing in column A, you should dynamically size you range to fit the data like this:

With Sheets("Liste Employé")
    Set valeurnorm = .Range("A2", .Cells(.Rows.Count, "A").End(xlUp)).Find(nos)
End With

Using Dynamic Named Ranges helps to give your code identity.  And that is what we want to do as developers.  If you have to comeback a year from now, you may not know what StandarValues represent but you will immediately understand what EmployeeNames are.

Set employeeNames= Sheets("Liste Employé").Range("EmployeeNames").Find(nos)

Alerts are necessary but having to click a MsgBox() can get irritating.  I would prefer that:

If .Find("Another Solution") is Not Nothing Then Call
Use(.Find("Another Solution"))

Solution 1: Use Conditional Formatting
How To Highlight Cells If Not In Another Column In Excel?
Solution 2: Use an ActiveX Combobox
Setup
• Insert a hidden ComboBox on each Worksheet that needs the name validation
• Give it a meaning full name
• Set it's ListFillRange a Dynamic Named Range of employee names

When a cell in the validation range is selected use the Workbook_SheetSelectionChange()
• Move the ComboBox over the Activecell
• REsize the ComboBox to fit the ActiveCell
• Set the ComboBox.LinkedCell = ActiveCell
• Set focus to the ComboBox
When the user selects a cell in the
• Hide the ComboBox
• Set the ComboBox.LinkedCell = Nothing

Private Sub Workbook_SheetSelectionChange(ByVal Sh As Object, ByVal Target As Range)
    AdjustEmployeeNameComboBox Sh, Target
End Sub

Private Sub AdjustEmployeeNameComboBox(ByVal Sh As Object, ByVal Target As Range)
    Dim hasEmployeeRange As Boolean
    On Error Resume Next

    Dim EmployeeNameComboBox As OLEObject
    Set EmployeeNameComboBox = Sh.OLEObjects("EmployeeNameComboBox")
    hasEmployeeRange = Err.Number = 0
    On Error GoTo 0

    If Not hasEmployeeRange Then Exit Sub

    Dim isEmployeeRangeSelected As Boolean
    isEmployeeRangeSelected = Not Intersect(Target, Sh.Range("C5:C8")) Is Nothing

    With EmployeeNameComboBox
        .LinkedCell = ""
        .Visible = isEmployeeRangeSelected
    End With

    If Target.Cells.Count > 1 Then Exit Sub

    If isEmployeeRangeSelected Then
        With EmployeeNameComboBox
            .LinkedCell = Target.Address
            .Top = Target.Top
            .Left = Target.Left
            .Height = Target.Height
            .Width = Target.Width
            .Activate
        End With
    End If
End Sub

Ugh..kinda nasty but it works.
Solution 3: Almost the Same as 2
Use the same setup as Solution2 but use a Cell Style to indicate that it is an Employee Name Validation Range.  The advantage of this is that you could have the validation anywhere on the Worksheet without having to update your code.
For this solution you will need to change

isEmployeeRangeSelected = Not Intersect(Target, Sh.Range("C5:C8")) Is Nothing

to this

isEmployeeRangeSelected = Target.Style = "Cell Style Name goes Here"


Area under curve and its volume as solid of revolution I would like to know if there is some more "elegant" way to write these 3 functions. Any tip or idea is welcome.

open System

module Calculus =
    let area f a b =
        let dx = 0.001
        seq { for x in a .. dx .. b -> (f x) * dx }
        |> Seq.sum

    let rotate f x =
        let y = f x
        y * y * Math.PI

    let volume f =
        area (rotate f)

[<EntryPoint>]
let main args =
    let f x = x * 2.0

    printfn "Running..."

    printfn "%f" (Calculus.area f 1.0 10.0)
    printfn "%f" (Calculus.volume f 1.0 10.0)

    0

Note

I understand the method for integration I am using here is not the best around, but it is not the point here. Let's focus on its implementation.

Here's how I would rewrite this:

First, pipe everything that you can. It's easier to follow the linear transition of a |> b |> c rather than c (b a).

Second, compose where possible. Compositions are powerful, and can allow you to abstract things away much more effectively.

Third, order function parameters by most -> least generic. We see f a b, but F# sees let f = f a; f b, so make sure your parameters are ordered by least-significant first.

module Calculus =
    let area f a b =
        let dx = 0.001
        { a .. dx .. b }
        |> Seq.sumBy (f >> (*) dx)

    let rotate f x =
        let multX = x |> f |> (*)
        Math.PI |> multX |> multX

    let volume = rotate >> area

[<EntryPoint>]
let main args =
    let shape = (2.0 |> (*), 1.0, 10.0)
    printfn "%f" (shape |||> Calculus.area)
    printfn "%f" (shape |||> Calculus.volume)

This ends up being three lines longer, but it's clearer. It's completely obvious what is happening, and the piping makes it easier to follow. We replaced the custom sequence generator with a basic one, and used sumBy to make our intent clearer. We want to sum f(x) * dx.

This also ends up one line shorter, and that's only because we rewrote volume as a pure composition, and pushed it to a one-line definition. I recommend doing this for things that are 2-3 steps. (So even let volume f = f |> rotate |> area would be acceptable.)

Instead of building the f in the main, we build the shape itself, and then pipe all three arguments of that shape to our area and volume methods. (Since the arguments are the same for each call.)

This means our shape is reusable, and we could build a function:

let printShape s =
    printfn "%f" (s |||> area)
    printfn "%f" (s |||> volume)

let shapes =
    let f = 2.0 |> (*)
    [(f, 1.0, 10.0); (f, 2.0, 10.0)]
shapes |> List.iter printShape

Now we can build more test-cases pretty easily and functionally. Because printShapes takes a single parameter of a (float -> float) * float * float, we can pass the entire shape to it and then pass each portion of the tuple as individual parameters.


Averaging a set of percentages between certain important levels I have a spreadsheet, called "To Do", with set of percentages in column E and importance levels in column A:

On another spreadsheet, I'm entering a formula to calculate the average of these percentages, but only those between certain importance levels:

See it on googledocs

This is the formula in question, indented for easier reading:

=TO_PERCENT(
     DIVIDE(
         SUMIF('To Do'!A3:A, "<2000", 'To Do'!E3:E)
       - SUMIF('To Do'!A3:A, "<=1000", 'To Do'!E3:E),
         MAX(
             COUNTIF('To Do'!A3:A, "<2000")
           - COUNTIF('To Do'!A3:A, "<=1000"),
             1
         )
     )
 )

Is there any better way to do this?

I originally wanted to use SUMIFS and COUNTIFS, but it said:

  error: Unknown function name

As @200_success pointed out in his comment, averaging percentages is a suspicious mathematical technique.

Each task in the TODO list could have its own weight, or relative value - a number that represents a chunk of progress towards "done"; then you can calculate a percentage by adding up the relative weights of all completed tasks (or more accurately, of a value derived from that task's %completed and weight), and dividing by the sum of the weights of all tasks.

I haven't played much with google-sheets, but I know excel pretty well; the formula you've come up with is exactly the kind of formula I'd have used in Excel 2003, before SUMIFS and COUNTIFS were introduced in Excel 2007.

When I enter a formula in a spreadsheet, I want to be able to copy that formula over to the next cell, without having to modify it. This involves a number of principles:

Don't hard-code cell references. In Excel I would have used names and/or tables - not sure google-sheets supports that, but in any case if none of that is supported you can still, and should, use absolute cell references - refer to 'To Do'!$A:$A and 'To Do'!$E:$E. Not sure what A3:A refers to.
Don't hard-code your variables. If each column is going to use a different set of [Priority] values, "<2000" and "<=1000" shouldn't be hard-coded. You can insert two rows above row 1, and put 1000 in row 1 and 2000 in row 2, so instead of "<2000" you'll have "<" & A$2, and instead of "<=1000" you'll have "<=" & A$1.

Lastly, I find

MAX(COUNTIF('To Do'!A3:A, "<2000") - COUNTIF('To Do'!A3:A, "<=1000"), 1)

is easier to read as

MAX(1, COUNTIF('To Do'!A3:A, "<2000") - COUNTIF('To Do'!A3:A, "<=1000"))

But the reason you're taking the MAX here, is to avoid a division by zero; by dividing by 1 whenever that's the case, you're showing mathematically incorrect results.

In Excel I'd wrap the division with an IFERROR, and return a string such as "-" when I'm dividing by zero. This accurately reports "this category is irrelevant", rather than "this category is 100% done".

Another simple option would be to add a column in the TODO list, to identify the actual priority level (which you currently have as a wasted row between each group); then a simple SUMIF / COUNTIF can do the trick: you only account for rows with a given [PriorityCode] value.

That's actually a "smell": your [Priority] values currently encode two values: the priority group, and the priority level, within that group.


The use of database "sets" rather than "functions" I've currently got a MSSQL stored procedure that uses multiple SQL functions to achieve a number of returns. Now I've been told about using sets rather than functions as it creates " bad code smell of a DBA who is not yet thinking in sets" Original Link
I'd like to better my code and love to know what sets are. So below are two of my functions from a long list of functions. Thanks to any that attempt the answer.
Function that returns a long list of values
CREATE FUNCTION dbo.funcGetPropertyEnviListFR
(
    @PropertyId AS INT
)
RETURNS VARCHAR(200)
AS
BEGIN
    DECLARE @ENVIFR VARCHAR(200)
    SET @ENVIFR = ''

        SELECT
            @ENVIFR = COALESCE(@ENVIFR + ', ', '') + ENVIRONMENT.FR
        FROM
            ENVIRONMENT
        INNER JOIN
            MATRIXPROPENVIRONMENT
        ON
            MATRIXPROPENVIRONMENT.EnvironmentId = ENVIRONMENT.id
        WHERE
            MATRIXPROPENVIRONMENT.PropertyId = @PropertyId
        ORDER BY
            Weight

        SET @ENVIFR = SUBSTRING(@ENVIFR, 3, 200)
        RETURN @ENVIFR
END

Result may look like - Coastal, Village, Habitat
Function that returns a seller's ID
CREATE FUNCTION dbo.funcSellIdByPropId
(
@PropertyId AS INT
)
RETURNS INT
AS
    BEGIN
        DECLARE @ID AS INT
        SET @ID = 0
            SELECT
        @ID = MATRIXAGENCYPROPERTY.AgencyId
    FROM
        MATRIXAGENCYPROPERTY
    WHERE
        MATRIXAGENCYPROPERTY.PropertyId = @PropertyId

    IF @ID = 0
        BEGIN
            SELECT
                @ID = PRIVATESELLERS.id
            FROM
                PRIVATESELLERS
            WHERE
                PRIVATESELLERS.PropertyId = @PropertyId
END

RETURN @ID
END

Result may look like - 43
Function that returns an image value from database
CREATE FUNCTION dbo.funcDefaultImage
(
    @PropertyId INT,
    @Position INT
)

RETURNS VARCHAR(50)

AS
BEGIN
    DECLARE @IMAGE VARCHAR(50)

    SELECT @IMAGE = T.image
    FROM (
        SELECT
            ROW_NUMBER() OVER (ORDER BY weight DESC) AS [rownum],
            PROPERTYGALLERY.image
        FROM
            PROPERTYGALLERY
        WHERE
            PROPERTYGALLERY.PropertyId = @PropertyId
    ) T
    WHERE
        rownum = @Position

    RETURN @IMAGE
END

Result may look like - property_0001.png

First goal in thinking in SQL (or thinking in sets) is to get rid of IF statements and Loops (or cursors).  So for your 2nd function this is much better:

CREATE FUNCTION dbo.funcSellIdByPropId
(
@PropertyId AS INT
)
RETURNS INT
AS
BEGIN
  DECLARE @ID AS INT

  SELECT @ID = COALESCE(MATRIXAGENCYPROPERTY.AgencyId, PRIVATESELLERS.id, 0)
  FROM  MATRIXAGENCYPROPERTY, PRIVATESELLERS
  WHERE MATRIXAGENCYPROPERTY.PropertyId = @PropertyId AND
        PRIVATESELLERS.PropertyId = @PropertyId

  RETURN @ID
END

This select can now be implemented as a view and then joined to any other select statement instead of calling a function.

For the 3rd function you could implement a view like this:

SELECT
  PropertyId
  ROW_NUMBER() OVER (PARTITION BY PropertyId ORDER BY weight DESC) AS [imgnum],
  image
FROM PROPERTYGALLERY

Now you can join on this view by PropertyId and imgnum instead of calling a function

For your first function take a look at this answer I posted on SO.  Using the XML generator like this is considered the fastest way to create a comma separated list in MS SQL.  Like the others examples I've given here, this technique could be used in a view to create a list of comma separated values partitioned by PropertyId

The key to both of these solutions is thinking in sets.  Instead of solving one problem (as the function does) they solve all the problems.  Then when you join to these queries you limit to just the data you need.

SQL Engines love this -- their optimizers are designed to make this kind of process work really fast.

Detailed Example (see comments)

First create the view:

-- MATRIXAGENCYPROPERTY.AgencyId and PRIVATESELLERS.id are mutually exclusive.
-- Grab all of them with their PropertyID
CREATE VIEW SellIDByPropertyID(ID, PropertyID) AS
(
  SELECT AgencyId, PropertyID
  FROM MATRIXAGENCYPROPERTY
  UNION ALL
  SELECT ID, PropertyId
  FROM PRIVATESELLERS
)

Now we can integrate it into your original query:

SELECT
   ...
   dbo.funcSellIdByPropId(T0.id) as SellerId,
   ...
FROM T0
   ...

To

SELECT
   ...
   S2P.ID,
   ...
FROM T0
JOIN SellIDByPropertyID S2P ON T0.id = S2P.PropertyId
   ...

Why this works

Once again the key to understanding is to think in sets.  If you just think about a single record a reaction might be the original SP would be faster because if it is a agency property it will only do one select and the IF statement will stop the 2nd select from being run giving a speed advantage.

  Side note: often on SQL Engines a select statement will be slower than
  an if statement so even the above assumption is wrong, but I'll ignore
  that for now.

However, this goes out the window as soon as we look at the set.  If the original query is returning more than one record -- say N records then with the original function call we have the overhead of N function calls + N Selects + N/2 Selects (if half are not agencies).  With the view we have just 2 Selects.  Clearly O(3.5N) > O(2)


Thread safe Parameters class (follow up) This is a follow up question to my previous question. I am trying to design a thread safe parameters class with only readable getters. Idea is it should be possible to initialize this class once with some parameters - and throughout app I should be able to query values. Deinitialization should also be possible.

Please find below. I am interested if this is thread safe or not, or how much thread safe it is?. This is my real code. Other code related feedback also welcome.

static class Parameters
{
  private static int connectionTimeout; // only getters of these will be visible to outside world
  private static int responseTimeout;
  private static bool isInit;
  private static readonly object Locker = new object();

  public static bool isInited()
  {
   lock(Locker)
    {
    return isInit;
    }
  }

  public static int ConnectionTimeout()
  {
    lock(Locker)
    {
    if(isInit == false) throw new Exception("init first");
    return connectionTimeout;
    }

  }

  public static int ResponseTimeout()
  {
    lock(Locker)
    {
    if(isInit == false) throw new Exception("init first");
    return responseTimeout;
    }

  }

  public static void init(int ct, int rt)
  {
    lock(Locker)
    {
      if(isInit) throw new Exception ("Already inited");
      isInit = true;
      connectionTimeout= ct;
      responseTimeout= rt;
    }
  }

  public static void Deinit()
  {

    lock(Locker)
    {
      if(!isInit) throw new Exception ("Already deinited");
      isInit = false;
      connectionTimeout= 0;
      responseTimeout= 0;
    }
  }

}

I agree with the comments.  I don't see the advantage of your code over something more like below.  If there is a use case you have in mind, please clarify.

public class Parameters
{
    public int ConnectionTimeout {get; private set;} // or a readonly field if you prefer
    public int ResponseTimeout {get; private set;}

    public Parameters(int ct, int rt)
    {
        ConnectionTimeout = ct;
        ResponseTimeout = rt;
    }
}

Threading concerns often disappear when you can use immutable data structures.


Find the elements that appear only once The task:

  Given an array of integers in which two elements appear exactly once
  and all other elements appear exactly twice, find the two elements
  that appear only once.

  For example, given the array [2, 4, 6, 8, 10, 2, 6, 10], return 4 and
  8. The order does not matter.

  Follow-up: Can you do this in linear time and constant space?

const lst = [2, 4, 6, 8, 10, 2, 6, 10];

My functional solution

const findUnique = lst => lst
  .sort((a,b) => a - b)
  .filter((x, i) => lst[i-1] !== x && lst[i+1] !== x);

console.log(findUnique(lst));

My imperative solution:

function findUnique2(lst) {
  const res = [];
  const map = new Map();
  lst.forEach(x => map.set(x, map.get(x) === undefined));
  map.forEach((val, key) => {
    if(val) { res.push(key); }
  });
  return res;
}

console.log(findUnique2(lst));

I think the imperative solution is in linear time but not constant space. How would you do it with constant space?

Sorting is not needed.  A Set may be a better data structure for the task.

const find2Unique2 = a => Array.from( a.reduce(
    (once, x) => (once.delete(x) || once.add(x), once),
    new Set()
))

console.log( find2Unique2( [2, 0, 6, 8, 10, 2, 6, 10] ) );

I"m not sure how to solve this in constant space.  If you xor all of the terms together

arr.reduce( (xor,x) => xor ^ x, 0 )

Then result = a ^ b, where a and b are your two unique terms.  If you can figure out a, then b = result ^ a.  Or you can find some other reduction, you'll have two equations and two unknowns, and an algebraic solution might be possible.

For example, xor of the negated array combines with positive xor to uniquely identify a and b if they are two bits wide:

  xor = arr.reduce( (xor,x) => xor ^ x, 0 )
  neg = arr.reduce( (xor,x) => xor ^ -x, 0 )

 a     b               xor   neg & 0xF (high bits omitted for clarity)
 0     1     =>          1  1111
 0    10     =>         10  1110
 0    11     =>         11  1101
 1    10     =>         11     1
 1    11     =>         10    10
10    11     =>          1    11

But it doesn't work for larger values.  You could do multiple passes, with each pass xor-ing a right-shifted version of elements that could possibly match (if the last two bytes are 11 and 10, you can skip numbers that don't end in those bits).

I didn't implement this because it's pretty complicated and it doesn't seem like the right answer.  But maybe it gives you an idea.


Simple calculator in Python This is my first ever Python program. I thought I would make a calculator that performs 5 operations: add, subtract, divide, multiply, power. I am aware Python has its own power function, called "pow", but I have defined my own power function for practice.

I've taken the input as a single string, and then I've tried to separate the numbers from the operator in my for loop, using what little Python I currently know. Perhaps this is the section of my code that could be made cleaner, above all else?

Coming from Java, my first instinct was to use a switch statement to handle each operator case. I quickly realised Python doesn't have switch statements, but I discovered that dictionaries are apparently a good substitute. But maybe there is a better way of handling each operator case?

And obviously, I should have error handling, but let's just ignore that detail for now.

val = str(input("Calculate: "))

# get first number, second number, and operator, from string
foundOp = False # has the operator been found?
a, b = "", "" # first and second number
for char in val:
    # concatenate digits
    if char.isdigit():
        if foundOp:
            b = b + char
        else:
            a = a + char
    # skip spaces
    elif char == " ":
        continue
    # find operator
    elif not char.isdigit():
        foundOp = True
        op = char

# convert a and b into int
a, b = int(a), int(b)

# compute a ^ b
def power(a,b):
    original_a = a
    for i in range(1,b):
        a *= original_a
    return a

calc = {
    "+": a + b,
    "-": a - b,
    "*": a * b,
    "/": a / b,
    "^": power(a,b)
}

print(calc[op])

Use functions

It's generally a good idea to sort your functionality into functions. Allows you for nice reuse and stuff like that.

string.split()

So how will we parse? Of course, you can parse every single input like you do, or even upgrade it to a tokenizer engine if you got a lot of spare time and effort. There's really 2 ways to go about there here. The simple one is string.split(). The complicated one is regexes, but you probably won't need these unless you're planning big.

Map to functions, not precomputed results

You'll notice I import functions from the operator builtin module. These do the exact same thing as the +, -, /, * and **(power) symbols, but we can use them as functions. This lets us put them in a dict we can create before we ask for user input. It also means we don't have to calculate all the operations, and can stick to only calculating the one we actually are interested in. Do note that this also obsoletes your power function, much as you could have done using ** yourself.

from operator import pow, sub, mul, add, truediv

def calculate():
    operators = {
        "+": add,
        "-": sub,
        "/": truediv,  # floordiv also exists, which is integer division
        "*": mul,
        "^": pow
    }
    val = input("Calculate: ")  # Do note that it's already a string, so you don't need to cast to one.
    for symbol, operator in operators.items():
        if symbol in val:  # Check if the symbol is in the string
            a, b = val.split(symbol)
            return operator(float(a), float(b))  # Cast them to floats - all builtin operators can handle them.

if __name__ == "__main__":
    print(calculate())

Last thing I added was what we call a guard. It makes you able to import this function from another script and use it, without having to run it.

If you want to go for bonus points, you can also switch to regexes for your input parsing.

Bonus info: Iterators

Python does a lot of smart things with iterators. For example, we're iterating over a dictionary here. A dictionary looks like this:

my_dict = {
    "key1": "value1",
    "key2": "value2",
    # etc...
}

It's a hashtable for the keys, and under the hood these are linked to pointers to the values. We can iterate over a dictionary in three ways: Over the keys, over the values, and over key: value pairs.

Keys

for key in my_dict:
    # "key1", "key2"....
# OR:
for key in my_dict.keys():
    # "key1", "key2"....

Values

for value in my_dict.values():
    # "value1", "value2"....

key/value pairs

for key, value in my_dict.items():
    # ("key1", "value1"), ("key2", "value2")....

In case of items(), the iterator gives a tuple of length 2 for every iteration. By assigning that tuple to key, value, we unpack it. This works the same as in the line:

a, b = 1, 2

On the RHS, we construct a 2-length tuple (1, 2), which is passed to the LHS, where it is unpacked to a, b. Since tuples enforce ordering, that's how python decides which value to put where.

See also: Python Docs Dictionary Looping Techniques


How many semi magic squares are there? I want to write a program to work out how many semi-magic squares there are.

Here is the definition of semi-magic squares

  If we define \$H_n(t)\$ is the number of semi magic squares which satisfy:

  The square is \$n*n\$ dimension
  The sum of each row elements and sum of each column elements equal to \$t\$

The first idea came to me is brute force searching. I know backtracing may be better, but I don't know how to write the program.

For example, when \$n = 3\$, \$t = 1\$.

The third constraint:

I need to clearly point out that all the matrix elements are non-negative integers. Apparently, we can get that elements \$\in[0,t]\$

According to this, we can get this table:

n\t      | 1|2|3|4|5|6|7|8
---------| -----
1        | 1|1|1|1|1|1|1|1
2        | 2|3|4|5|6|7|8|9
3        | 6|21|55|120|231|406|666|1035
4        | 24|282|2008|10147|40176|132724|381424|981541

When \$n = 3\$, it's something like this sequence.

#include<iostream>
using namespace std;
int square[3][3];

//check whether it is a semi-square
bool final_check(int n,int t)
{
    for(int i=0;i<n;i++)
    {
        int sum_col=0;
        int sum_row=0;
        for(int j=0;j<n;j++)
        {
            sum_row+=square[i][j];
            sum_col+=square[j][i];
        }
        if(sum_col!=t||sum_row!=t)
            return false;
    }
    return true;
}
void display(int n)
{
    for(int i=0;i<n;i++)
    {
        for(int j=0;j<n;j++)
            cout<<square[i][j]<<"\t";
        cout<<endl;
    }
    cout<<"************"<<endl;
}

//count \$H_3(1)\$
int main()
{
    int n=3,t=5;
    int total_num=0;
    for(int a=0;a<=t;a++)
    {
        square[0][0]=a;
        for(int b=0;b<=t;b++)
        {
            square[0][1]=b;
            for(int c=0;c<=t;c++)
            {
                square[0][2]=c;
                for(int d=0;d<=t;d++)
                {
                    square[1][0]=d;
                    for(int e=0;e<=t;e++)
                    {
                        square[1][1]=e;
                        for(int f=0;f<=t;f++)
                        {
                            square[1][2]=f;
                            for(int g=0;g<=t;g++)
                            {
                                square[2][0]=g;
                                for(int h=0;h<=t;h++)
                                {
                                    square[2][1]=h;
                                    for(int i=0;i<=t;i++)
                                    {
                                        square[2][2]=i;
                                        if(final_check(n,t))  //check
                                        {
                                            display(n);
                                            total_num++;
                                        }
                                    }
                                }
                            }
                        }
                    }
                }
            }
        }
    }
    cout<<"total number of semi magic square: "<<total_num<<endl;
    getchar();
    return 0;
}

I know that my code is very bad. Could anyone help me improve the code?  There are so many for loops. If \$n = 5\$, I would have to write 25 for loops.

Does anyone have a better solution?

There could be smarter ways to write your code in such a way that it would scale better (for instance, if you were to consider squares of side 4 or 5, you'd have 16 or 25 nested loops and local variables to handle).

First thing first, let's try to see what can easily be improved in your code.

You are considering all matrices of the following form :

 a b c
 d e f
 g h i

by making the different values go from 0 to t (included) and then check that a+b+c == d+e+f == g+h+i == a+d+g == b+e+h == c+f+i. However, things could be done in a much faster way : once a and b are fixed, you know that c == t - (a+b). Similarly, you can define f as t - (d+e). Then you can go one step further as g, h and i can also be determined using column values.

Once, this is done, the corresponding code becomes :

int main()
{
    int n=3,t=5;
    int total_num=0;
    for(int a=0;a<=t;a++)
    {
        square[0][0]=a;
        for(int b=0;b<=t;b++)
        {
            square[0][1]=b;
            int c = t - (a+b);
            if (c>=0)
            {
                square[0][2]=c;
                for(int d=0;d<=t;d++)
                {
                    square[1][0]=d;
                    for(int e=0;e<=t;e++)
                    {
                        square[1][1]=e;
                        int f = t - (d+e);
                        if (f >= 0)
                        {
                            square[1][2]=f;
                            int g = t - (a+d);
                            if (g >= 0)
                            {
                                square[2][0]=g;
                                int h = t - (b+e);
                                if (h >= 0)
                                {
                                    square[2][1]=h;
                                    int i = t - (g+h);
                                    assert(i == (t - (c+f)));
                                    if (i >= 0)
                                    {
                                        square[2][2]=i;
                                        if(final_check(n,t))  //check
                                        {
                                            display(n);
                                            total_num++;
                                        }
                                    }
                                }
                            }
                        }
                    }
                }
            }
        }
    }
    cout<<"total number of semi magic square: "<<total_num<<endl;
    return 0;
}

Once this is done, the final_check method becomes useless as all the generated squares are "magic".

Something else could easily be improved : you shouldn't use global variables as they make things hard to follow. In your case, it is quite easy to get rid of it. I also take this chance to re-organise the different loops/checks and change the limits for the loops so that we do not need to check that values are in the right ranger afterwards :

void display(int a, int b, int c, int d, int e, int f, int g, int h, int i)
{
    cout << a << "\t" << b << "\t" << c << endl;
    cout << d << "\t" << e << "\t" << f << endl;
    cout << g << "\t" << h << "\t" << i << endl;
    cout <<"************"<<endl;
}

/*
   a b c
   d e f
   g h i */

//count $H_3(1)$
int main()
{
    int t=5;
    int total_num=0;
    for(int a=0;a<=t;a++)
    {
        for(int b=0;b<=t-a;b++)
        {
            int c = t - (a+b);
            assert(c>=0);
            for(int d=0;d<=t-a;d++)
            {
                int g = t - (a+d);
                assert(g >= 0);
                for(int e=0;e<=t-d && e<=t-b;e++)
                {
                    int f = t - (d+e);
                    assert(f >= 0);
                    int h = t - (b+e);
                    assert(h >= 0);
                    int i = t - (g+h);
                    assert(i == (t - (c+f)));
                    if (i >= 0)
                    {
                        display(a, b, c, d, e, f, g, h, i);
                        total_num++;
                    }
                }
            }
        }
    }
    cout<<"total number of semi magic square: "<<total_num<<endl;
    return 0;
}

A few last things I'd like to tell :

using namespace std; is sometimes considered a bad practice. Some might say it is ok in a cpp file (by opposition to header file)
one could easily do with a few more comments
I am quite surprised that your code seems to assume that t is known.

This is as far as I can go re-using your code. A more interesting approach would use backtracking. You should be able to find already implemented algorithms using it to generate magic squares.


Customizing output in accordance to the input I wanted to know if I overdid it and maybe could have kept this code simpler and cleaner. I need to execute this assignment:

Write a program called multi_input.cpp that prompts the user to enter
several integers in any combination of octal, decimal, or hexadecimal,
using the 0 and 0x base suffixes; interprets the numbers correctly,
and converts them to decimal form. Then your program should output the
values in properly spaced columns like this:
      0x4 hexadecimal converts to     67 decimal
     0123 octal       converts to     83 decimal
       65 decimal     converts to     65 decimal

Now the code compiles and does what it's intended to do, but I still need to get into that programmer state of mind where you keep only the stuff that really matters, to the last letter (thus keeping the code simple, succinct and efficient). Showing me what I can get rid of and revise would bring me closer to achieving this.
#include"..\..\Header.h"

void print_number(const string& concatenation, const string& type) {
    istringstream s(concatenation);
    int number = 0;
    if(type == "hexadecimal") s >> hex >> number;
    if (type == "octal") s >> oct >> number;
    else s >> number;
    cout << concatenation << '\t' << type << "\tconverts to " << dec << number << " decimal.\n";
}

int main() {
    while (cin) {
        string da;
        cin >> da;
        if (da[0] == '0') {
            if (da[1] == 'x') print_number(da, "hexadecimal");
            else print_number(da, "octal");
        }
        else print_number(da, "decimal");
    }
    system("pause");
    return 0;
}

Header contains these lines (just in case):
    #include <algorithm>
#include <cmath>
#include <vector>
#include <iostream>
#include <sstream>
#include <fstream>
#include <string>

using namespace std;

void error(const string& a) {
    throw runtime_error(a);
}

Let's start off with a few general hints and tips:

First of all, as πάντα ῥεῖ already stated in the comments, using using namespace std is not a good idea, and doing that in a header if a lot worse. You're begging to introduce subtle bugs into your code as time goes on and things are added to your project and the standard libraries. Typing std:: a few times is not going to kill you, I'm sure.
#include"..\..\Header.h" is a code smell; relative paths in an include often indicate that your project structure is not good or your compile flags are not complete. You should specify all necessary include directories in the compiler invocation command line.
system("pause"); is not portable and should thus not be used unless absolutely necessary. Is this the case here? I don't think so. Make users of other operating systems (i.e. operating systems that aren't Microsoft Windows) happy and remove that line.
What happens if I enter the string "0" to your program? Whoops, undefined behavior! Why? if (da[1] == 'x') requires da to be at least two characters long, which in my case it simply doesn't happen to be. You should add a check to verify that this condition is satisfied.
Header.h seems redundant and superfluous. All of those includes should be at the top of your implementation file, not in the header. All includes that are not used (e.g. fstream) should be removed. The function error doesn't serve any real purpose, except obscuring the fact that something throws and exception, which can lead to people wondering where those exceptions come from. Also it isn't used, so it should be removed as well.

Now, let's talk about print_number in particular. One issue I have with this function is that type should never, ever be a string. You're wasting performance and causing binary bloat, when the fix would be to simply add an enum class with three members HEX, OCT and DEC. The other big issue is that this function limits its own usability by necessarily writing to std::cout. This has the potential to cause people using that function and wanting to separate outputs much grief, and is easily circumvented by having the function take a std::ostream& as parameter and writing to it instead. When invoking the function, you then simply pass std::cout as an argument.

Finally, this function does a little bit too much; two completely separate tasks, to be precise. One is taking a number in a given format and converting it to decimal, and the other is actually printing that number in a nicely formatted way. You should thus split it up into two functions: One which does the extraction, and one which does the output.


Converting anki-furigana to HTML-furigana without parser libraries I wrote a converter from anki-style furigana to HTML-style furigana. Eventually I want to do it as a parser, but as a first step I do it without. Furigana is used to write the pronunciation over certain characters in Japanese.

Example:

Anki would write it like this:

林[はやし]さんは 英語[えいご]は 話[はな]せます。

[]contains what's written over the char sequence in question and the char seq in question goes from the last ] to the next [. But if the next sequence begins only later (e.g. 英語), space is used to indicate that (\n works too and there is no nesting of brackets).
HTML has several ways, and I use this:

<ruby><rb>林<rt>はやし</ruby>さんは<ruby><rb>英語<rt>えいご</ruby>は<ruby><rb>話<rt>はな</ruby>せます。

A sequence and its pronunciation are enclosed by <ruby>, <rb> and <rt> to indicate what's on top and on the bottom.

The idea is that the string first gets converted into a sequence of tags that may hold text. That sequence is then grouped and after that the tags are converted to HTML.

I'm just wondering if it is OK so far.

import System.Environment

main :: IO ()
main = do
    args <- getArgs
    file <- readFile (head args)
    putStrLn (anki_2_html file)

anki_2_html = translate_2_html . group . parse'

data Tag = Kana String | Uptext String
     | Furigana String String
     | Blank | Newline

--convert string to tags
parse' :: String -> [Tag]
parse' string = parse string ""

parse :: String -> String -> [Tag]
parse [] []  = []
parse [] acc = [Kana acc]
parse (l:ls) ""  | l == '['  = parseUpText ls ""
                 | l == ' '  = Blank   : parse ls ""
                 | l == '\n' = Newline : parse ls ""
                 | otherwise = parse ls [l]

parse (l:ls) acc | l == '['        = Kana acc : parseUpText ls ""
                 | l `elem` " \n"  = Kana acc : parse (l:ls) ""
                 | otherwise       = parse ls (acc++[l])

parseUpText :: String -> String -> [Tag]
parseUpText  [] []  = []
parseUpText  [] acc = [Kana (' ':acc)]
parseUpText  (l:ls) []  | l == ']'  = parse  ls ""
                        | otherwise = parseUpText ls [l]

parseUpText  (l:ls) acc | l == ']'   = Uptext acc : parse  ls ""
                        | otherwise = parseUpText ls (acc++[l])

--group patterns of Tags
group :: [Tag] -> [Tag]
group (Blank   : Kana kj : Uptext ka : ms) = Furigana kj ka : group ms
group           (Kana kj : Uptext ka : ms) = Furigana kj ka : group ms
group (m:ms) = m : group ms
group [] = []

translate_2_html ::  [Tag] -> String
translate_2_html = foldl1 (++) . map m2s
          where
            m2s (Furigana below above) = "<ruby><rb>"++below++"<rt>"++above++"</ruby>"
            m2s (Kana k) = k
            m2s (Uptext f) = "<f>" ++ f ++ "</f>"
            m2s Blank  = " "
            m2s Newline= "<br>\n"

Building a list like ((... ++ [_]) ++ [_]) ++ [_] takes quadratic time, break doesn't.

parse :: String -> [Tag]
parse lss = case break (`elem` "[ \n") lss of
  ([],         []) -> []
  ([],     '[':ls) -> parseUpText ls
  ([],     ' ':ls) -> Blank     : parse ls
  ([],    '\n':ls) -> Newline   : parse ls
  (kana,       ls) -> Kana kana : parse ls

parseUpText :: String -> [Tag]
parseUpText lss = case break (== ']') lss of
  ([],         []) -> []
  (kana,       []) -> [Kana (' ':kana)]
  ([],     ']':ls) -> parse ls
  (uptext, ']':ls) -> Uptext uptext : parse ls


Printing pyramids in C (CS50) I'm taking the CS50 course, and we're asked to create a program which takes an input (the total height of the pyramid) and outputs a pyramid of that height.
i.e Here’s how the program might work if the user inputs 8 when prompted:
$ ./mario
Height: 8
       #  #
      ##  ##
     ###  ###
    ####  ####
   #####  #####
  ######  ######
 #######  #######
########  ########

Here is my code:
#include <cs50.h>
#include <stdio.h>

void print_hashes(int width, int empty_space);

int main(void)
{
    // Get height input between ranges 1 to 8 inclusive
    int height;
    do
    {
        height = get_int("Height: ");
    }
    while (height < 1 || height > 8);
    // Loops through each row
    for (int row = 1; row <= height; row++)
    {
        // Prints the left part of the pyramid, we need to print empty spaces before printing the hashes, so
        // the width is equal to the height, amount of hashes printed is equal to the row, so amount of empty spaces is the width - row
        print_hashes(height, height - row);
        printf("  ");
        // Prints the right part of the pyramid, we don't need to print empty spaces before printing hashes, so
        // the width is equal to the amount of hashes, i.e the row and the amount of empty space is just 0
        print_hashes(row, 0);
        printf("\n");
    }
}

void print_hashes(int width, int empty_space)
{
    // Loops through the column in the row
    for (int column = 0; column < width; column++)
    {
        // Checks if there needs to be an empty space within that column, if so print a space and if not print a hash
        if (column < empty_space)
        {
            printf(" ");
        }
        else
        {
            printf("#");
        }
    }
}

I would like some feedback on my code and how I could improve it; things like readability, efficiency, etc.
Is my code easily understandable? Am I overdoing the comments? The reason I put a lot of comments is because in my experience, when looking at code days/weeks after writing it, I tend to forget what it does. So I wrote explanations so that I can easily understand the code in the future.
How do I improve the style, and are there any code conventions I'm not following? Can I optimize this to make it a faster process? Or maybe there's a better way to do the same thing? Basically, how do I improve my code?

The code can be structured better, so the code is more self-documenting.
Self-documenting code doesn't need as many comments, which makes it easier on everyone. Comments that aren't there can't be obsolete either.

Take input (height)
Output pyramid

Those are the only lines I want to see in main. The rest should be in functions.
You started out nicely by creating a print_hashes function, but we can do better. For example, the entire do ... while can be in a function.
int ask_height(void)
{
    // Get height input between ranges 1 to 8 inclusive
    int height;
    do
    {
        height = get_int("Height: ");
    }
    while (height < 1 || height > 8);
    return height;
}

Now, we can start the main program like this instead:
int main(void)
{
    int height = ask_height();

That turns 7 lines in main into 1 line in main. And since the function name is descriptive, the comments are almost obsolete.
We can do the same thing with the rest of the program.
void print_pyramid(int height)
{
    // Loops through each row
    for (int row = 1; row <= height; row++)
    {
        // Prints the left part of the pyramid, we need to print empty spaces before printing the hashes, so
        // the width is equal to the height, amount of hashes printed is equal to the row, so amount of empty spaces is the width - row
        print_hashes(height, height - row);
        printf("  ");
        // Prints the right part of the pyramid, we don't need to print empty spaces before printing hashes, so
        // the width is equal to the amount of hashes, i.e the row and the amount of empty space is just 0
        print_hashes(row, 0);
        printf("\n");
    }
}

And all of a sudden, we have nicely split-out code, each function has one and only 1 thing to do (you may want to read up on the SRP, the Single-responsibility principle).
Now your file starts like this:
#include <cs50.h>
#include <stdio.h>

int ask_height(void);
void print_hashes(int width, int empty_space);
void print_pyramid(int height);

int main(void)
{
    int height = ask_height();
    print_pyramid(height);
}

That's it. The rest is in functions. The function names can probably be further improved, but I'm quite awful at that myself. If you want to go a step further, you don't even need a height variable in main. This works too:
int main(void)
{
    print_pyramid(ask_height());
}

The rest is in functions, and functions can be re-used. Besides, should you ever want to call your program from an outside location, that's a lot easier now too.


Recursing function to create dir in python I am trying to write a small function to safely create dir until dir until full path of file exists. I'm trying to do it in a recursing function and would love to get some feedback.

def makedir(filepath: str) -> bool:
    if os.path.exists(os.path.dirname(filepath)):
        return
    os.makedirs(os.path.dirname(filepath), exist_ok=True)
    makedir(filepath)

makedir('/a/b/c/d/x.text')
#should create a then b then c then d
makedir('/a/b/c/e/y.text')
#should create e only since a, b,c already exist

First, os.makedirs already creates all the intermediate directories in the path. That is its job. So there is no need to use recursion.

There is also no need to use os.path.exists, because you are using the exist_ok=True argument with os.makedirs. This means that os.makedirs will not error out if the directory already exists.*

Your type hint for the return type should be None, since you are not returning any values.

I would also change the name of your function. I think the name makedir is too close to os.makedirs, which is confusing.

import os

def makedirs_for_file(filepath: str) -> None:
    os.makedirs(os.path.dirname(filepath), exist_ok=True)

*Unless you are using a Python version before 3.4.1. In this case, os.makedirs may still error out, depending on the mode of the existing directory. See the documentation for more details.


Selecting employees and updating a DataGridView I've been reading up on different design patterns, mainly MVP and different adaptions of this design pattern / architecture.

Now, I've decided to have a little play with my own ideas, the main outcome separating the user view, from the actual modelling which does all the data changes.

The example is simple, the user is given three radio buttons to select from

Employee One
Employee Two
Employee Three

When these radio buttons are selected it changes a datagrid view with three columns

Firstname
Lastname
Age

My form code has three event driven functions, radioButton_checkChanged.

To start with, I programatically make some new employees: (Ignore the spelling mistake!)

string[] employeeArray = new string[3];
employeeData emplyeeOne = new employeeData("Bob", "Smith", "21");
employeeData emplyeeTwo = new employeeData("John", "Brown", "56");
employeeData emplyeeThree = new employeeData("Andy", "Guy", "28");

 private void employeeOne_CheckedChanged(object sender, EventArgs e)
        {
            updateEmployee(emplyeeOne);
            dataGridView1.Rows.Add(employeeArray);
        }

        private void employeeTwo_CheckedChanged(object sender, EventArgs e)
        {
            updateEmployee(emplyeeTwo);
            dataGridView1.Rows.Add(employeeArray);
        }

        private void employeeThree_CheckedChanged(object sender, EventArgs e)
        {
            updateEmployee(emplyeeThree);
            dataGridView1.Rows.Add(employeeArray);
        }

employeeData class (I've tried to go for encapsulation on this):

public class employeeData
    {
        private string classFirstName { get; set; }
        private string classLastName { get; set; }
        private string classAge { get; set; }

        public employeeData(string firstname, string lastname, string age)
        {
            classFirstName = firstname;
            classLastName = lastname;
            classAge = age;
        }

        public string[] updatePerson()
        {
            string[] data = new string[3];
            data[0] = classFirstName;
            data[1] = classLastName;
            data[2] = classAge;
            return data;
        }
    }

updateEmployee function as seen earlier:

 private void updateEmployee(employeeData who)
    {
        dataGridView1.Rows.Clear();
        dataGridView1.Refresh();
        employeeArray = who.updatePerson();
    }

Am I on to the beginning of any major software design patterns here? Any adaptations and or improvements you can see to make my code a more robust?

Naming is important, C# uses Pascal casing for classes and methods, so you should use EmployeeData instead of employeeData, and UpdatePerson instead of updatePerson
Fields live in classes and thus you don't need to give them a class prefix

private string FirstName { get; set; }
private string LastName { get; set; }
private string Age { get; set; }

C# has a type system that you can use instead of declaring everything as a string

private int Age {get;set;}

updatePerson uses the verb update, and methods with verbs usually do/change things with side-effects but your method just converts the object to a string[], and to be honest, I don't see any need for this method.


Listening for values changed by the memory I'm fetching some values from the memory. I would like to create some custom events that I can listen to for each value. Since there are no existing events regarding memory changes, I've decided to go with a polling solution. I'm restarting the timer below manually just to make certain the work is finished.

Would this be an okay solution (considering the circumstances) to my problem? Do you see any issues with creating multiple timers? As far as I know, each instance will run on its own thread.

This was the best I could come up with right off the bat. I would like to hear your input regarding possible improvements:

var vc = new ValueChange(1000, Blah.GetMemoryValue());
vc.PropertyChanged += OnPropertyChanged;

void OnPropertyChanged(object sender, PropertyChangedEventArgs e)
{
    Console.WriteLine("prop change");
}

public class ValueChange : INotifyPropertyChanged
{
    private int _val;
    private readonly System.Timers.Timer _timer;

    private int Value
    {
        set
        {
           _val = value;
            RaisePropertyChanged("Val");
        }
        get
        {
            return _val;
        }
   }

   public ValueChange(double polingInterval, int val)
   {
        _val = val;

       _timer = new System.Timers.Timer { AutoReset = false, Interval = polingInterval };
       _timer.Elapsed += TimerElapsed;
       _timer.Start();
   }

   private void TimerElapsed(object sender, ElapsedEventArgs e)
   {
       var newValue = Blah.GetMemoryValue();

       if (Value != newValue)
       {
           Value = newValue;
       }

        _timer.Start();

   }

   public event PropertyChangedEventHandler PropertyChanged;

    private void RaisePropertyChanged(string caller)
    {
       if (PropertyChanged != null)
       {
           PropertyChanged(this, new PropertyChangedEventArgs(caller));
       }
    }
}

Having a private property is quite unusual. It's especially weird since you only use it once in TimerElapsed. I would probably just use the field directly and call RaisePropertyChanged() manually when necessary.
Why is the initial value passed in the constructor? I think the class should retrieve the initial value by itself.
PropertyChanged should be raised for a property that's readable from the outside and the name in the event args should be the same as the name of that property.
Your code is a prime candidate for making it more generic: you could change ValueChange to ValueChange<T> and int _val to T _val. (But if you do this, be careful with equality, !object.Equals(_val, newValue) is probably the simplest way to do it correctly.)

Also, Blah.GetMemoryValue should be configurable by the caller. You could achieve this by accepting a Func<T> delegate in the constructor. Something like:

private readonly Func<T> _getValue;

public ValueChange(double polingInterval, Func<T> getValue)
{
    _getValue = getValue;

     …
}

private void TimerElapsed(object sender, ElapsedEventArgs e)
{
    var newValue = _getValue();

    …
}

I like to avoid null checks on events by making sure there always is at least one subscriber:

public event PropertyChangedEventHandler PropertyChanged = delegate { };

But this is not really a common technique.
Creating multiple timers is okay, they execute on thread pool threads. This means that if several timers tick at the same time, they will be (most likely) executed concurrently by different threads.
In general, polling like this is not ideal. It would be better if you could get a notification whenever the value actually changes. But it seems like this is not possible in your case.
You shouldn't shorten the names of your variables. So, _value is better than just _val.


Egyptian fractions in Python Here's my code for the Egyptian fraction written in Python. I need your feedback if there is any room for improvement.

def eg_fr(n, d):
    ls = []
    ls_str = []
    fraction = n / d
    total = 0
    for i in range(1, 1000):
        ls.append((1/i, str(1) + '/' + str(i)))
    for i in ls:
        while i[0] + total <= fraction:
            ls_str.append(i[1])
            total += i[0]
            if total >= fraction:
                break
    print('%s/%s = ' % (n, d), end='')
    for i in range(len(ls_str) - 1):
        print('', ls_str[i], '+ ', end='')
    print(ls_str[-1], end='')
    return ls_str

if __name__ == '__main__':
    eg_fr(7, 133)

Names

What should your code do? Neither your code (by good names) nor your documentation inside the code (docstrings) nor your question does state the task in detail. So I cannot tell if your code is delivering the right results (but I don't think it does). So fix your names, there is no need to save characters. The bigger the scope the better the names have to be. Also add some docstring with a more detailed description of the task the function tries to solve.

Correctness of the results

>>> _ = eg_fr(7, 133)
7/133 = 1/19
>>> _ = eg_fr(7, 132)
7/132 = 1/19

That surprises at least me. At least the print statement lies to me. there is no equality in the second expression

Functions returning values vs. I/O

>>> eg_fr(7, 133)
7/133 = 1/19
['1/19']

Functions usually should either return values and have no other side effects like print or they should do tasks with side effect and return nothing (None) or success/error codes only. Your function does both. Either call the function print_egypti... and remove the return value or do remove the print from inside the function.

I strongly suggest to do printing outside the algorithm to have a testable function that is not cluttering the output.

Printing

A single test is no test.

If you did

if __name__ == '__main__':
    eg_fr(7, 133)
    eg_fr(7, 132)

you would have noticed not only the interesting results but also that you missed a line feed.

Looping

In Python you never do

for i in range(len(something)):
    do(something[i])

You always do

for element in something:
    do(element)

in your case

for element in ls_str[:-1]:
    print('', element, '+ ', end='')

that is less error prone.

String concatenation

You do concatenate the string output via printing in a loop. Use join() here. Instead of

for element in ls_str[:-1]:
    print('', element, '+ ', end='')
print(ls_str[-1])

you do

print(' + '.join(ls_str))

Testability

As mentioned before we split algorithm from I/O to get a nice and testable function

def egyptian_fractions(nominator, denominator):
    # [...]

def print_egyptian_fractions(nominator, denominator):
    print('{}/{} = '.format(nominator, denominator), end='')
    print(' + '.join(egyptian_fractions(nominator, denominator)))

Go for purity

We squeeze out all stuff from the algorithm that does not belong there. That is the string representation and also the nominator which is always 1. The final algorithm shall return only a list of denominators.

The remaining algorithm is

def egyptian_fractions_(nominator, denominator):
    denominators = []
    fraction = nominator / denominator
    total = 0
    for i in range(1, 1000):
        while 1/i + total <= fraction:
            denominators.append(i)
            total += 1/i
            if total >= fraction:
                break
    return denominators

The fitting print function is

def print_egyptian_fractions(nominator, denominator):
    ls_str = ['1/' + str(e) for e in egyptian_fractions(nominator, denominator)]
    print('{}/{} = '.format(nominator, denominator), end='')
    print(' + '.join(ls_str))

Algorithm

Depending on your task, I cannot know for sure, your algorithm is erroneous and inefficient.

You do prepare a lengthy fractions for a later test where you could calculate straightforward.
you do prepare string representations where you could leave this to the output functions
You limited yourself to a smallest fraction of 1/1000. Why?
You use float numbers for calculation and testing inside your algorithm and will get precision errors

I should mention there is the module fractions that is providing fractional math. As this is an exercise we will try to do it by hand.

First we try to lay out the algorithm

We try to find the biggest fraction f that is smaller or equal to the given fraction g
We keep the denominator from the fraction f

3a. we subtract fraction f from the given fraction g to get the remainder

So far this is what you did, now the change

3b. we try to find a fractional expression for the remainder (nominator, denominator)

we repeat from step 1

For step 1 you did a simple loop testing to find the biggest fraction. We do it a little more advanced, we divide the given denominator by the given nominator to find the new egyptian denominator. We use integer division for that step to work without precision loss. If the remainder of this division is 0 we are done, if not we have to increment the new denominator.

For step 3 we do some basic math to do the subtraction with the help of a common denominator to avoid precision loss and stick to a nominator/denominator fraction format.

What do we do for step 4? the task is the very same - solve the same problem for a new pair of nominator/denominator. We do a simple recursion and call the same function again appending that result to that from the current recursion depth.

def egyptian_fractions(nominator, denominator):
    if nominator == 1:
        return [denominator]    # exactly fitting fraction (was normalized)
    else:
        d = denominator // nominator
        r = denominator % nominator
        if r == 0:
            return [d]          # exactly fitting fraction (wasn't normalized)
        else:
            d += 1              # increment to get the biggest fitting fraction
                                # keep the result and add the result of the subproblem
            return [d] + egyptian_fractions(nominator*d-denominator, denominator*d)


Python implementation of find_all_indexes I got asked this question before in an interview.

For example, find_all_indexes('ababc', 'abc') returns 2. This Python function should find the index of first occurrence pattern. I also wrote a function that returns a list of starting indexes of all occurrences of the pattern in a text.

I have the following implementation that handles most of the cases:

def find_all_indexes(text, pattern):
    indexes = []
    text_index = 0
    while text_index != len(text):
        if pattern == '':
            indexes.append(text_index)
        else:
            for i in range(len(pattern)):
                if text_index + i < len(text):
                    if text[text_index + i] != pattern[i]:
                        break
                    if i == len(pattern) - 1:
                        indexes.append(text_index)
        text_index += 1
    return indexes

def test_string_algorithms(text, pattern):
    indexes = find_all_indexes(text, pattern)
    print('find_all_indexes({!r}, {!r}) => {}'.format(text, pattern, indexes))

def main():
    test_string_algorithms(text, pattern)
    print('Usage: {} text pattern'.format(script))
    print("find_all_indexes('abra cadabra', 'abra') => [0, 8]")

if __name__ == '__main__':
    main()

I am not sure if I exhausted all of the edge cases, but I ran this unit test, and it passed all tests.

The pytest output:

pytest test.py
======================= test session starts ========================
platform darwin -- Python 2.7.12, pytest-3.0.5, py-1.4.32, pluggy-0.4.0

collected 3 items

test.py .........

===================== 3 passed in 0.07 seconds

The code looks correct but overly complicated for the naïve algorithm used.

For a first refactor, you could pull out the special case pattern == ''. (We might pull it back in later).

Then

            for i in range(len(pattern)):
                if text_index + i < len(text):
                    if text[text_index + i] != pattern[i]:
                        break
                    if i == len(pattern) - 1:
                        indexes.append(text_index)

is just a substring equality check. There are two things to note here:

Since it fails when text_index + len(pattern) > len(text) the outer loop runs too far.
This could be written as if text[text_index:text_index+len(pattern)] == pattern: indexes.append(text_index).

Thirdly, the outer while loop could be written as a for in range, which is more Pythonic.

Putting those together, we have

def find_all_indexes(text, pattern):
    if pattern == '':
        return range(len(text) + 1)

    indexes = []
    for text_index in range(len(text) - len(pattern) + 1):
        if text[text_index : text_index + len(pattern)] == pattern:
            indexes.append(text_index)
    return indexes

But then the Pythonic way to write a for if append loop like that is a comprehension:

def find_all_indexes(text, pattern):
    if pattern == '':
        return range(len(text) + 1)

    return [text_index for text_index in range(len(text) - len(pattern) + 1)
            if text[text_index : text_index + len(pattern)] == pattern]

And now the special case is really a microoptimisation, and could possibly be removed to leave a one-liner.

However, that's not the final word. Naïve string search is asymptotically inferior to techniques such as Knuth-Morris-Pratt string search. You probably shouldn't roll your own KMP routine, but you could use string.find to get the benefit. So a better implementation would be a loop around string.find.