Do-While loop in Python, with limit on the amount of 'do's I'm in a location with poor Internet access. In order to ensure that a call to an external service succeeds, I am wrapping the call in a pseudo-do-while in Python. Additionally, I am limiting the call to running a maximum of three times:

def post_safe(url, params):

    done = 0
    max_tries = 3
    messages = []

    while done<max_tries:
        try:
            response = requests.post(url, data=params)
        except Exception as e:
            messages.append(e)
            time.sleep(1)
        else:
            done = max_tries+1
        done += 1

    if done==max_tries:
        output = "%s\n" % (datetime.now().strftime('%Y-%m-%d %H:%M'),)
        output+= "requests() failed 3 times:\n"
        for m in messages:
            output+= m+"\n"
        print(output)

        return False

    return True

I personally am not satisfied with the 'Pythoness' of this code. It looks inelegant, and I would have preferred to use the type bool for done but that would require a second variable to count the iterations.

Is there a cleaner way to achieve a do-while in Python while limiting the amount of iterations?

Python has a language feature just for that: else clauses on loops.

  Loop statements may have an else clause; it is executed when the loop terminates through exhaustion of the list (with for) or when the condition becomes false (with while), but not when the loop is terminated by a break statement.

def post_safe(url, params):
    max_tries = 3
    messages = []

    for _ in range(max_tries):
        try:
            response = requests.post(url, data=params)
            return True
        except Exception as e:
            messages.append(e)
            time.sleep(1)

    else:
        print(datetime.now().strftime('%Y-%m-%d %H:%M'))
        print("requests() failed %d times:" % (max_tries))
        for m in messages:
            print(m)
        return False

I'd also make the following changes:

The else clause on the try block is unnecessary; it can be replaced with a break within the try block just after calling .post().  But in the end, you'll just end up returning True, so why not get out of there right away with return True?  (For that matter, the else block that I just suggested is also unnecessary, as the else could be removed and its contents outdented by one level.)
It seems easier to print each line separately than to build a multi-line string to print all at once.  You might want to consider logging instead.


Wrapping packet information I am developing a network application and therefor need to specify the packets I want to send. I have a basic interfaces class and one for the packets that implement them.

public interface IPacket
{
}

public interface IServerPacket : IPacket
{
}

public interface IServerChatPacket : IServerPacket
{
    ServerChatPacketType Type { get; }
}

public interface IServerAccountPacket : IServerPacket
{
    ServerAccountPacketType Type { get; }
}

The specification of an IServerPacket is needed for easier and faster serialization. Anyways, now I can use these sub-interfaces for chat server and account communication with the connected clients.

[Serializable]
public struct ServerChatIncomingMessagePacket : IServerChatPacket
{
    public readonly string text;

    public ServerChatPacketType
    {
        get { return ServerChatPacketType.IncomingMessage; }
    }

    public ServerChatIncomingMessagePacket(string text)
    {
        this.text = text;
    }
}

[Serializable]
public struct ServerAccountAnswerLoginPacket : IServerAccountPacket
{
    public readonly bool success;

    public ServerAccountPacketType
    {
        get { return ServerAccountPacketType.AnswerLogin; }
    }

    public ServerAccountAnswerLoginPacket(bool success)
    {
        this.success = success;
    }
}

Is this the right way to do it? Currently I really have no problems with this, but maybe I am using the interfaces or structs wrong? I am not new into C# or OOP but there may be some way that solves this in a different manner or with better performance (structs have a chance to hold much data which is unpractical for this type kind)?

One empty interface could be considered a code smell, but can sometimes be useful for reflection purposes. Having two is nonsensical. An interface defines a contract that the type has to adhere to. Where's the contract here? I fail to see how these two types belong to the same inheritance tree. "Type" members also set off alarm bells in my head. They're usually an indication that there are missing interfaces. You could then just inspect the actual type, if you really had to.

I would go with something more like this.

public interface IMessagePacket
{
    string Message { get; }
}

public interface IStatusPacket
{
    bool Status { get; }
}

[Serializable]
public struct ServerChatIncomingMessagePacket : IMessagePacket
{
    public readonly string text;

    public string Message
    {
        get { return text; }
    }

    public ServerChatIncomingMessagePacket(string text)
    {
        this.text = text;
    }
}

[Serializable]
public struct ServerAccountAnswerLoginPacket : IStatusPacket
{
    public readonly bool success;

    public bool Status
    {
         get { return success; }
    }

    public ServerAccountAnswerLoginPacket(bool success)
    {
        this.success = success;
    }
}

This allows us to define behavior of the types. We could easily implement an OutGoingMessagePacket that implements IMessagePacket, or a LogOffPacket that implements IStatusPacket.

On another note, in your original implementation, you never used the parameter you passed to ServerAccountAnswerLoginPacket's constructor, or it's related private readonly field.


Solving jumbled letters using a given dictionary Given a dictionary, a method to do lookup in dictionary and a M x N board where every cell has one character. Find all possible words that can be formed by a sequence of adjacent characters.
Example:

Input: dictionary[] = {"GEEKS", "FOR", "QUIZ", "GO"};
       boggle[][]   = {{'G','I','Z'},
                       {'U','E','K'},
                       {'Q','S','E'}};

Output:  Following words of dictionary are present
         GEEKS, QUIZ

Below is my code:

dictSize=int(raw_input())
dictionary=raw_input().split(' ')
m,n=map(int,raw_input().split(' '))
boggle=[[0 for x in range(n)] for y in range(m)]
#print dictionary

bString=raw_input().split()
for i in range(0,m):
    for j in range(0,n):
        boggle[i][j]=bString[(i*n)+j]
#print boggle

lookUp={}
for i in range(0,m):
    for j in range(0,n):
        lookUp[boggle[i][j]]=lookUp.get(boggle[i][j],0)+1

possibility=[]
flag=0
for i in range(0,len(dictionary)):
    for s in dictionary[i]:
        if(lookUp.has_key(s)):
            flag=0
        else:
            flag=1
    if(flag==0):
        possibility.append(dictionary[i])
#print possibility

for i in range(0,len(possibility)):
    print possibility[i],

Loop like a native

I highly recommend Ned Batchelder's excellent talk called "Loop like a native".

for i in range(0,len(possibility)):
    print possibility[i],

can be rewritten:

for p in possibility:
    print p,

Python 2 in 2018

It is probably a bad idea to write new Python 2 code in 2018. You should try to write it in Python 3.


Stopping a running thread I am trying to stop an executing runnable instance after the run method has been called.

I have come up with the below approach

    public class StoppableWorkflowTask implements Runnable {
        volatile Thread runner = null;

        @Override
        public void run() {
            runner = Thread.currentThread();
            try {

                while (true) {
                    System.out.println("Stop Thread " + Thread.currentThread().getId());
                }

            } catch (Exception ex) {
                ex.printStackTrace();
            }

        }

        void stop() {
            runner.stop();
        }

    }

public class StoppableWorkflowTaskTest {
    public static void main(String[] args) {
        StoppableWorkflowTask stoppableTask = new StoppableWorkflowTask();

        Thread thread = new Thread(stoppableTask);

        thread.start();
        System.out.println(thread.currentThread().getId());

        try {
            stoppableTask.stop();
        } catch (Exception ex) {
            ex.printStackTrace();
        }

    }
}

As per Java documention it is unsafe to call thread.stop(). I have checked few examples , but all either use thread.sleep() which can be terminated on interrupt.

The recommended way from Oracle is Thread.interrupt().  For example:

public void stop() {
    Thread thread = runner;
    runner = null;
    thread.interrupt();
}

public void run() {
    runner = Thread.currentThread();
    while (runner != null) {
        System.out.println("Stop Thread " + Thread.currentThread().getId());
    }
    System.out.println(" Done ");
}

It is true that sleep() can be interrupted (which will raise an InterruptedException), which is what the interrupt() will do.  However, afterwards the Thread can continue doing whatever it wants after being “interrupted”, so you must also check for a stop-condition of some kind.  Above, we check runner != null.  But as you can see above, you don’t need a sleep() in the worker thread.

If you don’t want to run the Thread in a loop, or you have many different loops where the task may spin, you will have to make the stop check in each of those places.


Check for invalid name I'm trying to write a regular expression to check names for invalid entries. Basically I'm trying to eliminate people entering random junk as their name. I know I can't completely eliminate it but I'm trying to make it a little harder at the very least. Here is what I have so far. It works fine I'm wondering if there's a way to improve this.

[b-df-hj-np-tv-z]{5,}|[aeiou]{4,}|([a-z])(?=([a-z]*\1){3,})|([ ])(?=([a-z]*\3){2,})|[0-9]+

First off, there is an error: You have the part [b- duplicated at the beginning.

The first section matches five successive consonants, which is possbile. Examples for German words (although not names) with up to eight successive consonants: http://www.duden.de/sprachwissen/sprachratgeber/die-woerter-mit-den-meisten-aufeinanderfolgenden-konsonanten

The same goes for four successive vowels. Again examples in German with up to five: http://www.duden.de/sprachwissen/sprachratgeber/die-woerter-mit-den-meisten-aufeinanderfolgenden-vokalen

Then the next one basically matches when a specific letter is repeated three times anywhere in the name and that can't be terribly rare.

And finally one that only allows two spaces, thus names consisting of three words. And I've come across plenty of names in my live with more than three words.

Finally a technical note: If you really want to do this, you need to do it server side. JavaScript validation is trivially easy to circumvent.


Reverse the digits of an Integer Reverse digits of an integer.

Example1:

x = 123,

return 321
Example2:

x = -123,

return -321

Return 0 if the result overflows and does not fit in a 32 bit signed integer

This is my approach:

public class Solution {
    public int reverse(int A) {

        String ans = " ";
        String finalAns = " ";
        int range = 2147483647;
        if( (A > 0) && (A <= range))
            {
                ans = String.valueOf(A);
                return (Integer.parseInt(new StringBuilder(ans).reverse().toString()));
            }
        if( (Math.abs(A) > range))
            return 0;

        else if ( (A > - (range + 1)) && (A < 0) )
            {
                ans = String.valueOf(Math.abs(A));

                if( Math.abs(A) > range)
                    return 0;

                finalAns =  (new StringBuilder(ans).reverse().toString());

                double d = Double.parseDouble(finalAns);
                if( d  == (int)d )
                    return Integer.parseInt( "-" + finalAns);
                else
                    return 0;
            }
        else
            return 0;
    }
}

I have following questions regarding my code:

1) Is there a better approach to solve this question?

2) Are there any Java coding conventions that I have violated gravely?

3) Am I doing redundant steps?

Reference

If I were presented with that solution as an answer to an interview question, I would definitely question your judgement and taste, even if the solution worked correctly — which it doesn't.

The value for range, 2147483647, is a magic number, better written as Integer.MAX_VALUE.  However, even if (A > 0) && (A <= range) is true, the reversed result might not fit in an int, in which case Integer.parseInt(…) would throw a NumberFormatException.

I have previously advised you to avoid using floating-point operations when dealing with integer arithmetic.  If your goal is to handle int overflow, then use long.

In any case, using StringBuilder.reverse() is, in my opinion, a bad idea.  CPUs are very efficient at arithmetic, but less adept at handling strings.  Both String.valueOf(int) and Integer.parseInt(String) do a lot of work behind the scenes.  Though the inefficiency is quite tolerable for all practical purposes, I would consider the solution to have missed the mark, if judged as an interview solution.

I'd be happier with a purely arithmetic solution like this, which should be far more efficient, and that demonstrates your ability to write a loop:

public int reverse(int a) {
    long reversed = 0;
    while (a != 0) {
        reversed = 10 * reversed + (a % 10);
        a /= 10;
    }
    return (reversed == (int)reversed) ? (int)reversed
                                       : 0;    // int overflow detected
}

Note that the overflow detection is also much simpler.  There are no special cases — we simply check whether downcasting from long to int can be safely done.  Also note that (a % 10) produces a negative result when a is negative, so that no special handling is needed for negative numbers either.


Replacing non-ASCII characters I wrote a C# program to remove non-ASCII characters in a text file, and then output the result to a .NonAsciiChars file.

The input file is in XML format. In fact, the data may all be on two lines, which is why I am not doing the replacement line by line. Instead, I'm using StreamReader.ReadToEnd().

The problem is, the input file can be as big as 4 GB. When this happens, I'm getting the following OutOfMemoryException:

DateTime:2014-08-04 12:55:26,035 Thread ID:[1] Log Level:ERROR Logger Property:OS_fileParser.Program property:[(null)] - Message:System.OutOfMemoryException: Exception of type 'System.OutOfMemoryException' was thrown.
   at System.Text.StringBuilder.ExpandByABlock(Int32 minBlockCharCount)
   at System.Text.StringBuilder.Append(Char* value, Int32 valueCount)
   at System.Text.StringBuilder.Append(Char[] value, Int32 startIndex, Int32 charCount)
   at System.IO.StreamReader.ReadToEnd()
   at OS_fileParser.MyProgram.FormatXmlFile(String inFile) in D:\Test\myProgram.cs:line 530
   at OS_fileParser.MyProgram.Run() in D:\Test\myProgram.cs:line 336

Line 530 contains content = Regex.Replace(content, pattern, "");, while line 336 calls a method with the following body:

const string pattern = @"[^\x20-\x7E]";

string content;
using (var reader = new StreamReader(inFile))
{
    content = reader.ReadToEnd();
    reader.Close();
}

content = Regex.Replace(content, pattern, "");

using (var writer = new StreamWriter(inFile + ".NonAsciiChars"))
{
    writer.Write(content);
    writer.Close();
}

using (var myXmlReader = XmlReader.Create(inFile + ".NonAsciiChars", myXmlReaderSettings))
{
    try
    {
        while (myXmlReader.Read())
        {
        }
    }
    catch (XmlException ex)
    {
        Logger.Error("Validation error: " + ex);
    }
}

How can I improve the memory footprint of my code?

You need to use the two Streams as, well, streams: read a manageable part of the input, transform it, write it to the output and repeat.

int bufferSize = 4096; // or whatever

char[] characters = new char[bufferSize];

using (var reader = new StreamReader(inFile))
using (var writer = new StreamWriter(inFile + ".NonAsciiChars"))
{
    while (true)
    {
        int read = reader.Read(characters, 0, characters.Length);

        if (read == 0)
            break;

        var replaced = Regex.Replace(new string(characters), pattern, string.Empty);

        writer.Write(replaced);
    }
}

Some notes about this code:

Notice the missing Close() calls: the whole point of using is safe closing of streams, and similar resources, so you don't need to close them twice.
This code (just like the original) creates a lot of garbage to be collected by the GC. Since your regex is actually very simple, it might be better to manually work directly with char[]s.
I used string.Empty instead of "". This makes it very clear that empty string was actually intended and it's not just “I started writing string and then forgot about it” bug.


Best practice for URL's architectural Design in Django I have been designing websites with Django for a while and I have also designed various sites. But here is a question that comes to mind at the beginning of any new project:
What is the best URLs architectural Design that creates both logical and meaningful URLs path as well as pluggable apps?
I will explain my question with a practical example.
Practical Example And Problem
Suppose that we have project with these features:

It has three applications called Shop, Blog and Support.

It has three URL section:
2.1. Public section: that start with /.
2.2. Consumer panel: that start with /panel.
2.3. Administrator panel: that start with /administrator.

Each application has 3 views. For example Shop has: public_shop_view, panel_shop_view and administrator_shop_view.
Now, What is the best URL design for this project? I describe two possible answers here for URLs design.
Solution 1:
project/urls.py
path('', include('Shop.urls', namespce='Shop')),
path('', include('Blog.urls', namespce='Blog')),
path('', include('Support.urls', namespce='Support')),

Shop/urls.py
path('shop/', views.public_shop_view, name='public_shop_view'),
path('panel/shop/', views.panel_shop_view, name='panel_shop_view'),
path('administrator/shop/', views.administrator_shop_view, name='administrator_shop_view'),

Blog/urls.py
path('blog/', views.public_blog_view, name='public_blog_view'),
path('panel/blog/', views.panel_blog_view, name='panel_blog_view'),
path('administrator/blog/', views.administrator_blog_view, name='administrator_shop_view'),

Support/urls.py
path('support/', views.public_support_view, name='public_support_view'),
path('panel/support/', views.panel_support_view, name='panel_support_view'),
path('administrator/support/', views.administrator_support_view, name='administrator_support_view'),

Solution 2:
In this solution we create three apps (Public, Panel, Administrator) that just have urls.py file (to follow the interface strategy):
project/urls.py
path('', include('Public.urls', namespce='Interface')),
path('panel/', include('Panel.urls', namespce='Panel')),
path('administrator/', include('Administrator.urls', namespce='Administrator')),

Public/urls.py
from Support import views as support_views
from Blog import views as blog_views
from Shop import views as shop_views

path('support/', support_views.public_support_view, name='public_support_view'),
path('blog/', blog_views.public_blog_view, name='public_blog_view'),
path('shop/', shop_views.public_shop_view, name='public_shop_view'),

Panel/urls.py
from Support import views as support_views
from Blog import views as blog_views
from Shop import views as shop_views

path('support/', support_views.panel_support_view, name='panel_support_view'),
path('blog/', blog_views.panel_blog_view, name='panel_blog_view'),
path('shop/', shop_views.panel_shop_view, name='panel_shop_view'),

Administrator/urls.py
from Support import views as support_views
from Blog import views as blog_views
from Shop import views as shop_views

path('support/', support_views.administrator_support_view, name='administrator_support_view'),
path('blog/', blog_views.administrator_blog_view, name='administrator_blog_view'),
path('shop/', shop_views.administrator_shop_view, name='administrator_shop_view'),

Now, What is the best practice? Solution 1 or solution 2 or other solution?

I've read the comments and am a bit torn by the discussion... yes, this code is not runnable; yes, you assume background knowledge; but you do that because 90% of Django starter code is not relevant to this discussion, but is essential to a Django project, so I think this is worth a shot.
For starters, it's worth pointing out that which of these solutions you choose has no impact on the code you write, since you will always refer to URL's by their name rather than by their path. This is solely a matter of beauty for how your capabilities are exposed to users and external services.
For this case, I would suggest a refactor that separates the front-end (paths) from the back-end (functionality). Have an app each for support, blog, and store, each of which implement their own models, serializers, utility functions, Celery tasks, etc. Then, have an app each for panel and administration that implements the paths, views, and user-facing logic about how people interact with those underlying concepts. You end up with some internal dependencies, but they're bipartite (only apps in the "frontend" group depend on only apps in the "backend" groups) and acyclic, so that's not a big deal.
If designed this way, you get the best of both worlds. You can keep the logical separation you show above among isolated logical components, but can use clean path construction and view implementation designed around how the users interact with your system. The downside is a larger number of apps and some internal dependencies, but those dependencies become explicit and you avoid the apps needing to implicitly know about each other and knowing how to avoid path conflicts.


Bad performance bash, changing display brightness I have a script that changes the display brightness using xrandr. It is my first bash script. I mainly had a lot of trouble with dealing with the floating point values. I think that is one of the main reasons the performance is bad.

  1 #!/bin/bash
  2 # increase/decrease display brightness of xrandr of both displays
  3 # use:
  4 # $ ./change_brightness [up/down (0/1)]
  5
  6 export current_brightness=$(xrandr --verbose | awk '/Brightness/ { print $2; exit }')
  7
  8 export increase=$1
  9 export new_brightness=$current_brightness
 10
 11 if [ "$increase" = "1" ]; then
 12     if [ $current_brightness != "1.0" ]; then
 13         new_brightness="$current_brightness + 0.2"
 14     fi
 15 elif [ "$increase" = "0" ]; then
 16     if [ $current_brightness != "0.20" ]; then
 17         new_brightness="$current_brightness - 0.2"
 18     fi
 19 fi
 20
 21 new_brightness=$(bc <<< "$new_brightness")
 22
 23 xrandr --output eDP-1-1 --brightness $new_brightness
 24 xrandr --output DP-0 --brightness $new_brightness

The performance isn't terrible, but any improvements would be highly appreciated. Also the improvements that have nothing to do with performance.

I'm confused about your goal for this question:

performance is bad
performance isn't terrible
improvements would be appreciated
improvements have nothing to do with performance.

Nevertheless, code review notes:

you don't have to export every variable, only the ones that are needed by processes you spawn from this script.
validate inputs: why do anything if the input is neither "0" nor "1"?

Here's my take:

#!/bin/bash
# increase/decrease display brightness of xrandr of both displays
# use:
# $ ./change_brightness [up/down (0/1)]

current_brightness=$(xrandr --verbose | awk '/Brightness/ { print $2; exit }')
case $1 in
    0) direction=-1;;
    1) direction=1;;
    *) echo "some error message"; exit 1;;
esac

export current_brightness direction
new_brightness=$( perl -E '
    use List::Util qw( min max );
    say max(0, min($ENV{current_brightness} + $ENV{direction} * 0.2, 1.0));
')

xrandr --output eDP-1-1 --brightness $new_brightness
xrandr --output DP-0 --brightness $new_brightness


Digital root recursive function Haven't any experience writing recursive functions.(Generally tend to avoid them - not sure if that's a negative). What do you think about the following?
This is also my first time using JUnit, I'd appreciate input if I'm doing something wrong there.

Challenge: Create a digital Root Function.
Specifications:
A digital root is the recursive sum of all the digits in a number.
Given n, take the sum of the digits of n.
If the resulting value has two digits, continue reducing until a single-digit number is produced.
This is only applicable to the natural numbers.

Solution:
public static int getDigitalRoot(int num) {
        if (Integer.toString(num).length() == 1) { return num; }
        int result = 0;

        for (char c : Integer.toString(num).toCharArray()) {
            result += Character.getNumericValue(c);
        }

        return getDigitalRoot(result);
    }

Tests:
@Test
    public void Tests() {
      assertEquals(6 , Utilities.getDigitalRoot(942));
      assertEquals(7, Utilities.getDigitalRoot(16));
    }

With a bit of knowledge of modular math then you can know that for numbers > 0 getDigitalRoot(n)== n%9 except when dividable by 9 then it is 9.

This simplifies the algorithm a bit.

public static int getDigitalRoot(int n) {
    if(n==0)
        return 0;
    if(n%9 == 0)
        return 9;
    return n%9;
}

If you still want the looped version you can do a variation; instead of using another variable to hold the result and retrying until smaller than 10 just do this:

while (num >= 10) {
    num = num/10 +num % 10;
}

It works for roughly the same reason as the first modulo operation.


New Haskell package: OpenCL I am developing a high level OpenCL binding in Haskell, and I need peer-review and testing. It currently only gets platform and device info from OpenCL.

I have lots of functions that only change the returned type and the size of the type passed to the C library, but I don't know how to fix it.

GitHub

getDeviceInfoUlong :: CLDeviceInfo_ -> CLDeviceID -> IO (Either CLError CLulong)
getDeviceInfoUlong infoid device = alloca $ \(dat :: Ptr CLulong) -> do
  whenSuccess (raw_clGetDeviceInfo device infoid size (castPtr dat) nullPtr)
    $ peek dat
    where
      size = fromIntegral $ sizeOf (0::CLulong)

getDeviceInfoSizet :: CLDeviceInfo_ -> CLDeviceID -> IO (Either CLError CSize)
getDeviceInfoSizet infoid device = alloca $ \(dat :: Ptr CSize) -> do
  whenSuccess (raw_clGetDeviceInfo device infoid size (castPtr dat) nullPtr)
    $ peek dat
    where
      size = fromIntegral $ sizeOf (0::CSize)

Looking through the code, I see often the "high level wrapper" returns CLuints and such. I would say that is improper - Why not return the haskell type?

Instead of returning, say, CLint, do

f :: Integral i => ... -> i

so that the user doesn't have to litter his code with those types and fromIntegrals all over the place.

Also, don't leave commented code in the repository. You can always recover via git if you need to.


Set a specific bit to a specific value This is the code I came up with:

void set_bit_at_pos(int* num, int pos, bool val)
{
    if (val) {
        int mask = 1 << pos;
        *num |= mask;
    }
    else {
        int mask = ~(1 << pos);
        *num &= mask;
    }
}

I'm new to bitwise operations, so this approach may not be the best. Nevertheless any suggestions are appreciated.

For the following main:

int main()
{
    int num = 5;
    cout << num << endl;
    set_bit_at_pos(&num, 1, true);
    cout << num << endl;
}

(I know about using namespace std;)

This is the output:

5
7

This is simpler:

#include <stdint.h> // #include <cstdint> in C++

void write_bit_32b(uint32_t *num, uint8_t pos, bool val)
{

        *num &= ~(UINT32_C(1) << pos);
        *num |= (uint32_t)val << pos;
}

Use fixed-width integers if you can (see above).

Use int main(void):

C17::6.11.6:

  Function declarators The use of function declarators with empty
  parentheses (not prototype-format parameter type declarators) is an
  obsolescent feature.

This may be interesting to you:

https://stackoverflow.com/q/47981/6872717


Solving the max-subarray problem from CLRS in C++ I'm working through Algorithms Cormen et al to learn more about algorithms and modern C++.  I've implemented the maximum subarray problem and would like some feedback.  I'd really like to use the modern additions to C++ effectively, so please let me know where things can be improved.

One of my concerns was using std::numeric_limits::infinity in the code.  Would this be used in production code?  How would one handle something like infinity otherwise?  I often see Dijkstra's algorithm described in pseudocode this way (using infinity), but is there something smarter/more standard to use?

Another concern was using tuples to return multiple values from a function.  I find that I then have to use std::get quite a lot to access the individual elements of the tuple.  Is this acceptable?

#include <iostream>
#include <tuple>
#include <vector>
#include <limits>
#include <math.h>

std::tuple <int,int,double> max_crossing_subarray(std::vector<double> A, int low, int mid, int high);
std::tuple<int,int,double> max_subarray(std::vector<double> A, int low, int high);

int main(){

    std::vector<double> A = {13, -3, -25, 20, -3, -16, -23, 18, 20, -7, 12, -5, -22, 15, -4, 7};
    //std::vector<double> A = {1, -1, 9};

    std::tuple<int,int,double> answer = max_subarray(A,0,A.size()-1);

    std::cout << std::get<0>(answer) << '\n' << std::get<1>(answer) << '\n' << std::get<2>(answer) << std::endl;

    return 0;

}

std::tuple<int,int,double> max_subarray(std::vector<double> A, int low, int high){

    std::tuple<int,int,double> ans;
    std::tuple<int,int,double> left_sub;
    std::tuple<int,int,double> right_sub;
    std::tuple<int,int,double> crossing_sub;

    if(high==low){
        ans = {low,high,A[low]};
        return ans;
    }
    else{
        int mid = floor((low+high)/2);
        left_sub = max_subarray(A, low, mid);
        right_sub = max_subarray(A, mid+1, high);
        crossing_sub = max_crossing_subarray(A, low, mid, high);
    }

    // is my left subarray the largest?
    if(std::get<2>(left_sub)>std::get<2>(right_sub) & std::get<2>(left_sub)>std::get<2>(crossing_sub)){
        ans = left_sub;
    }
    //is my right subarray largest?
    else if (std::get<2>(right_sub)>std::get<2>(left_sub) & std::get<2>(right_sub)>std::get<2>(crossing_sub)){
        ans = right_sub;
    }
    // my crossing subarray is largest
    else{
        ans = crossing_sub;
    }

    return ans;

}

std::tuple <int,int,double> max_crossing_subarray(std::vector<double> A, int low, int mid, int high){

    double inf = std::numeric_limits<double>::infinity();  // is this wise?
    double sum = 0;

    int max_left = 0;
    double left_sum = -inf;
    for(int i=mid; i>=low; i--){
        sum += A[i];
        if(sum>left_sum){
            left_sum = sum;
            max_left = i;
        }
    }

    int max_right = 0;
    double right_sum = -inf;
    sum = 0;
    for(int j=mid+1; j<=high; j++){
        sum += A[j];
        if(sum > right_sum){
            right_sum = sum;
            max_right = j;
        }

    }

    std::tuple<int,int,double> out = {max_left, max_right, left_sum+right_sum};

    return out;

}

I'd really like to use the modern additions to C++ effectively, so please
  let me know where things can be improved.

C++ is suprisingly strong in expressing algorithms in terms of minimal type
requirements quite "easily" with templates. To effectively use the standard
library and its modern additions one should know that

C++ algorithms are written in terms of iterators (or ranges)

So I would like to C++-ify your implementation of max_subarray and show the
usage of some new and cool features of C++17.

Your current version only works for std::vector and with little adaption to
any array-like container. But what if you want to search a std::list or some
container like std::unordered_map (with some projection onto keys or values)?

With that abstraction I wouldn't call the algorithm max_subarray but
max_subrange. The fundamental algorithm is max_crossing_subrange and that
is why I will show you how to C++ify this function

  One of my concerns was using std::numeric_limits::infinity in the code.
  Would this be used in production code? How would one handle something like
  infinity otherwise?

You are right to question that, because it is not okay or generic in any way.
What if you don't sum numbers but you are folding some lists instead in your
accumulation of neighboured elements?
What you do there is a simple search for a maximum and you could simply
evaluate the first expression instead. Look as an example at std::max_element, to see it is done there.

Just to answer your last concern

  Another concern was using tuples to return multiple values from a function.
  I find that I then have to use std::get quite a lot to access the individual
  elements of the tuple. Is this acceptable?

Yes, that is okay and should be optimized away completely.

Now lets start step-by-step with a generic mas_crossing_subrange algorithm.

Warning: This does not lead to the optimal algorithm (which is O(N)) but just shows how to express algorithms in a C++-way for your example.

Your max_crossing_subarray uses two loops which are essentially same but reversed in their direction. In fact you can express it as one function using std::reverse_iterator. Lets refactor a sub function which performs the loop. An Iterator version would look like this:

/// \brief Fixing the first position we search an end position which
///        maximises the sum from first to end.
template <
    typename I,
    typename T = value_type_t<I>
>
constexpr std::pair<I, T>
max_sum_not_empty(I first, I last, const T& init = T{})
noexcept
{
    assert(first != last);
    auto max = std::make_pair(first, *first);
    auto current_sum = max.second;
    for (auto iter{std::next(first)}; iter != last; ++iter) {
        current_sum = current_sum + *iter;
        if (max < current_sum) {
            max = std::make_pair(iter, current_sum);
        }
    }
    return max;
}

But this isn't totally generic either. What if we want to generalise the accumulation to not just sums but any given fold operator f: X x X -> X (we can not use f: Y x X -> Y in max_crossing) where summation and less-comparison are default behaviour. It would look like this:

template <
    typename I,
    typename T = value_type_t<I>,
    typename F = std::plus<>,
    typename P = std::less<>
>
constexpr std::pair<I, T> max_accumulation_not_empty(
        I first, I last, const T& init = T{}, F op = F{}, P pred = P{})
noexcept (
    std::is_nothrow_callable_v<F(const T&, reference_t<I>)> &&
    std::is_nothrow_callable_v<P(const T&, const T&)>
)
{
    assert(first != last);
    auto max = std::make_pair(first, std::invoke(op, init, *first));
    auto current_sum = max.second;
    for (auto iter{std::next(first)}; iter != last; ++iter) {
        current_sum = std::invoke(op, current_sum, *iter);
        if (std::invoke(pred, max.second, current_sum)) {
            max = std::make_pair(iter, current_sum);
        }
    }
    return max;
}

The reversed algorithm can be written as

template <
    typename I,
    typename T = value_type_t<I>,
    typename F = std::plus<>,
    typename P = std::less<>
>
constexpr std::pair<I, T> reverse_max_accumulation_not_empty(
        I first, I last, const T& init = T{}, F op = F{}, P pred = P{})
noexcept (
    std::is_nothrow_callable_v<F(const T&, reference_t<I>)> &&
    std::is_nothrow_callable_v<P(const T&, const T&)>
)
{
    auto rfirst = std::make_reverse_iterator(last);
    auto rlast = std::make_reverse_iterator(first);
    auto [rbest, sum] = max_accumulation_not_empty(rfirst, rlast, init,
                std::ref(op), std::ref(pred));
    auto best = std::next(rbest).base();
    return std::make_pair(best, sum);
}

Your max_crossing algorithm can be then given as

template <
    typename I,
    typename T = value_type_t<I>,
    typename F = std::plus<>,
    typename P = std::less<>
>
constexpr std::tuple<I, difference_type_t<I>, T>
max_crossing_subrange(I first, I mid, I last, const T& init = T{},
        F op = F{}, P pred = P{})
noexcept (
    std::is_nothrow_callable_v<F(const T&, reference_t<I>)> &&
    std::is_nothrow_callable_v<P(const T&, const T&)>
)
{
    assert(first != mid && mid != last);
    auto [best_left, left_sum] = reverse_max_accumulation_not_empty(
            first, mid, init, std::ref(op), std::ref(pred));
    auto [best_right, right_sum] = max_accumulation_not_empty(
            mid, last, init, std::ref(op), std::ref(pred));
    auto length = std::distance(best_left, best_right);
    // here comes a strong assumption: op(left_sum, right_sum) needs
    // an operator op: X x X -> X
    return {best_left, length, op(left_sum, right_sum)};
}

If this would lead to an optimal algorithm it would look like this in production code. As an exercise: Try to finish this and proof that the assertions always evaluates to true.


Lexicographically largest string from allowed swaps Problem: Given a string str and array of pairs that indicates which
  indices in the string can be swapped, return the lexicographically
  largest string that results from doing the allowed swaps. You can swap
  indices any number of times.

  Example

  For str = "abdc" and pairs = [[1, 4], [3, 4]], the output should be
  swapLexOrder(str, pairs) = "dbca".

  By swapping the given indices, you get the strings: "cbda", "cbad",
  "dbac", "dbca". The lexicographically largest string in this list is
  "dbca".

My comments: My program passed all the tests on the CodeSignal website, but since I am relatively new to algorithms, there's probably a more efficient way to solve it. In addition, I'm looking for general code review: does my code have signs that I look like a beginner? thank you.

#include<iostream>
#include<set>
#include<string>
#include<vector>
#include<map>
#include<algorithm>

std::string swapLexOrder(std::string str, std::vector<std::vector<int>> pairs)
{
    if(pairs.size() == 0) return str;
    std::vector<std::set<int>> pairpool; //pairpool : contains sets of interchangeable indices
    std::vector<std::vector<std::string>> stringpool; //stringpool : contains vectors of interchangeable characters

    //creates pairpool structure
    for(std::size_t i = 0; i < pairs.size(); i++)
    {
        bool alrExists = false;
        std::set<int> newSet;
        for(auto& p : pairpool)
        {
            for(auto ele : p)
            {
                if((pairs[i][0] == ele) || (pairs[i][1] == ele))
                {
                    if(!alrExists)
                    {
                        alrExists = true;
                        p.insert(pairs[i][0]);
                        p.insert(pairs[i][1]);
                        newSet = p;
                    }
                    else
                    {
                        if(p == newSet) break;
                        p.insert(newSet.begin(), newSet.end());
                        pairpool.erase(std::remove(pairpool.begin(), pairpool.end(), newSet), pairpool.end());
                        break;  //  needed this break statement really badly
                    }
                }
            }
        }
        if(!alrExists)
        {
            newSet.insert(pairs[i][0]);
            newSet.insert(pairs[i][1]);
            pairpool.push_back(newSet);
        }
    }

    //creates sorted stringpool structure
    for(auto p : pairpool)
    {
        std::vector<std::string> newset;
        for(auto ele : p)
        {
            newset.push_back(str.substr(ele - 1, 1));
        }
        std::sort(newset.begin(), newset.end());
        stringpool.push_back(newset);
    }

    //uses stringpool and pairpool to modify string only one time through
    int counter = 0;
    for(auto p : pairpool)
    {
        for(auto ele : p)
        {
            str.replace(ele - 1, 1, stringpool[counter].back());
            stringpool[counter].pop_back();
        }
        counter++;
    }

    return str;
}

int main()
{
    std::cout << swapLexOrder("acxrabdz", {{1, 3}, {6, 8}, {3, 8}, {2, 7}}) << "\n";
    std::string STR = "lvvyfrbhgiyexoirhunnuejzhesylojwbyatfkrv";
    std::vector<std::vector<int>> PAIR =
    {
        {13, 23},
        {13, 28},
        {15, 20},
        {24, 29},
        {6, 7},
        {3, 4},
        {21, 30},
        {2, 13},
        {12, 15},
        {19, 23},
        {10, 19},
        {13, 14},
        {6, 16},
        {17, 25},
        {6, 21},
        {17, 26},
        {5, 6},
        {12, 24}
    };
    std::cout << swapLexOrder(STR, PAIR) << "\n";
    return 0;
}

Your code is quite good for a beginning, but there's still much room for improvement. Your code main flaw is that it is difficult to read. There are also some performance issues, but they are a lot easier to correct.
N.B: I'll stay close to your algorithm, although you could probably find a better one in a graph library (see for instance https://en.wikipedia.org/wiki/Connected_component_(graph_theory)#algorithms)
Performance
You must pick your types more carefully. Don't choose a type that allocates memory when you can use one that doesn't. For instance, don't represent a pair of indices by a std::vector: a pair will always have two elements, there is no need to ever grow it. So choose a std::pair<int, int> instead: it is lighter, quicker, and reflects your intent more clearly. In the same way, don't use std::string to represent 1 character: your stringpool can as well be of type std::vector<std::string>.
You must also be more careful while passing objects around. Don't pass large objects by value unless you need both to copy and modify them. For instance, the signature: std::string swapLexOrder(std::string str, std::vector<std::vector<int>> pairs) is only partially correct. You will indeed modify str, and you want the user to keep his original string untouched, so passing by value is a good call. But pairs is read-only in your code, so pass it by const reference.
Expressiveness
Your code is one big function containing a lot of nested loops. As a result,it's very hard to understand, let alone optimize. There is two ways to fix it: the first is to better comment on your algorithm (for instance, a minimum would be: first, we merge all pairs of indices that are connected together; then we extract the corresponding characters from the input string, sort them in decreasing order and put them back); the second, is to make your code express your intent clearly.
The most important rule, which also is one of the hardest to obey, is to choose meaningful names. newset doesn't mean much, neither does p. pairpool should not contain set of more than two elements.
Another way to write more expressive code is to avoid "raw loops". Use named algorithms instead. For instance, I rewrote the first part of your algorithm as a combination of a partition discriminating sets containing one element of the considered pair and an accumulation which merges them:
std::string swapLexOrder(std::string str, const std::vector<std::pair<int, int>>& pairs)
{
    if(pairs.size() == 0) return str;
    std::vector<std::set<int>> index_permutations; //pairpool : contains sets of interchangeable indices

    for(auto pair : pairs)
    {
        // [matches, end) <- sets containing either one of the pair's elements
        auto matches = std::partition(index_permutations.begin(), index_permutations.end(),
                                      [&pair](const auto& permutation_set) {
            return std::none_of(permutation_set.begin(), permutation_set.end(), [&pair](auto index) {
                return index == pair.first || index == pair.second;
            });
        });

        if (matches == index_permutations.end()) index_permutations.push_back({pair.first, pair.second});
        else { // merge matches
            *matches = std::accumulate(matches, index_permutations.end(),
                                       std::set<int>{pair.first, pair.second},
                                       [](auto&& init, auto&& permutation) {
                init.insert(permutation.begin(), permutation.end());
                return init;
            });
            index_permutations.erase(std::next(matches), index_permutations.end());
        }
    }
// ...

Here's a link to the partially rewritten code if you want to experiment with it: https://wandbox.org/permlink/6Wij6Y4NuooMlW6C


Finding the second largest value In an Array findSecondLargestNumberInTheArray() method returns the second highest number from an array.

Could you please review this code to enhance performance.

public static int findSecondLargestNumberInTheArray(int array[]) {
    // Initialize these to the smallest value possible
    int highest = Integer.MIN_VALUE;
    int secondHighest = Integer.MIN_VALUE;

    // Loop over the array
    for (int i = 0; i < array.length; i++) {

        // If current element is greater than highest
        if (array[i] > highest) {

            // assign second highest element to highest element
            secondHighest = highest;

            // highest element to current element
            highest = array[i];
        } else if (array[i] > secondHighest)
            // Just replace the second highest
            secondHighest = array[i];
    }

    // After exiting the loop, secondHighest now represents the second
    // largest value in the array
    return secondHighest;
}

Several comments are unnecessary:

    // Loop over the array
    for (int i = 0; i < array.length; i++) {

    // assign second highest element to highest element
    secondHighest = highest;

I recommend do use braces { ... } for all if-blocks, even if that
consists only of a single statement, and definitely if there is
an additional comment line:

        } else if (array[i] > secondHighest)
            // Just replace the second highest
            secondHighest = array[i];

Iteration over all array elements can be simplified with an
“enhanced for loop:”

    for (int elem : array) {
        // ...
    }

Your function returns Integer.MIN_VALUE for an empty or single-element
array, which is fine if specified as such. An alternative would be
to throw an exception for arrays with less than two elements.

With respect to performance, I doubt there is much to improve.
The function iterates once over all array elements, which looks pretty
much optimal.


When values are pasted into a bigger range, the values should be repeated as needed to fill the range I am currently working on a project, and I just wrote a piece of it that requires a lot of array manipulation.  Given a text input I want to expand it to fill the number of rows and columns I have.  I worked out a solution, but I am concerned that I am creating too many arrays in my solution.  I am looking for pointers on how I can make this code more efficient.  I always get really lost when trying to refactor things like this.

function fillIn(data,numRows,numCols) {
  // First, split the data by newlines and tabs into an array of arrays.
  var dataArr = data.split('\n').map(function(row) { return row.split('\t'); });

  function fillRows(pRows){
    return [].concat.apply([], new Array(Math.floor(numRows / pRows.length)).fill(pRows));
  }

  function fillCols(pRows) {
    for (var i = 0; i < pRows.length; i++) {
      pRows[i] = [].concat.apply([], new Array(Math.floor(numCols / pRows[i].length)).fill(pRows[i]));
    }
    return pRows;
  }

  function fillSelection(pRows) {
    if (numRows > pRows.length) {
      pRows = fillRows(pRows);
    }
    if (numCols > pRows[0].length) {
      pRows = fillCols(pRows);
    }
    return pRows;
  }

  dataArr = fillSelection(dataArr);

  return dataArr;
}

console.log(fillIn('foo',2,2)); //[['foo','foo'],['foo','foo']]
console.log(fillIn('foo\tbar',4,2)); //[['foo','bar'],['foo', 'bar'],['foo','bar'],['foo','bar']]
console.log(fillIn('foo\nbar',2,4)); // [['foo','foo','foo','foo'],['bar','bar','bar','bar']]

ECMAScript 6

If you're able to use ECMAScript 6 then arrow functions could help clean up some of your code.

var dataArr = data.split('\n').map(row => row.split('\t'));

Unit Testing

You're logging the results, but it would be extremely helpful for your refactoring process to create some test cases for your function.

function test(input, expected) {
  var result = fillIn(input.data, input.numRows, input.numCols);

  function matrixEquals(m1, m2) {
    if (m1.length !== m2.length) return false;

    for (var r = 0; r < m1.length; r++) {
      if (m1[r].length !== m2[r].length) return false;

      for (var c = 0; c < m1[r].length; c++) {
        if (m1[r][c] !== m2[r][c]) return false;
      }
    }

    return true;
  }

  if (!matrixEquals(expected, result)) {
    throw "Test Failed for: input=" + JSON.stringify(input) +
      ", result=" + JSON.stringify(result);
  }
}

test({
  data: "foo\tbar",
  numRows: 2,
  numCols: 2
}, [
  ["foo", "bar"],
  ["foo", "bar"]
]);

test({
  data: "foo\tbar\nfat\tcat",
  numRows: 4,
  numCols: 4
}, [
  ["foo", "bar", "foo", "bar"],
  ["fat", "cat", "fat", "cat"],
  ["foo", "bar", "foo", "bar"],
  ["fat", "cat", "fat", "cat"]
]);

If you write your test cases well, you'll be able to tell if you broke your function simply by running the test cases.

Do you really even need to create all the arrays?

You could just extend the initial array directly:

function fillIn(data, numRows, numCols) {
  var matrix = data.split('\n').map(row => row.split('\t'));

  var missingRows = numRows - matrix.length;
  for (var r = 0; r < missingRows; r++) {
    matrix.push(matrix[r].slice());
  }

  var missingCols = numCols - matrix[0].length;
  for (var c = 0; c < missingCols; c++) {
    matrix.forEach(function(row) {
      row.push(row[c]);
    });
  }

  return matrix;
}


Calculating area of different figures on a case by case basis My program looks for the area of the figures, depending on the value of the variable а.

area -> triangle

area -> triangle2

area -> square

area -> rhombus
I want to get constructive criticism about the structure of the code and what can be redone. The program is written in the Python programming language.

while True:
    class S():
        print("Для выхода нажмите Enter")
        a = int(input("1. Площадь прямоугольного тр. \n2. Площадь равнобедренного тр. \n3. Площадь квадрата \n4. Площадь ромба  "))
        if a == 1:
            print("Это нахождение площади прямоугольного треугольника")
            a1 = float(input("Введите первый катет  "))
            b1 = float(input("Введите второй катет  "))
            class Triangle1():
                    def  __init__(self, a1, b1):
                        self.a = a1
                        self.b = b1
                    def area(self):
                        return (self.a * self.b) / 2
            Striangle1 = Triangle1(a1,b1)
            print("Площадь равна = ",Striangle1.area())
        elif a == 2:
            print("Это нахождение площади равнобедренного треугольника")
            a2 = float(input("Введите основание  "))
            b2 = float(input("Введите высоту  "))
            class Triangle2():
                    def  __init__(self, a2, b2):
                        self.a2 = a2
                        self.b2 = b2
                    def area(self):
                        return (self.a2/2)*self.b2
            Striangle2 = Triangle2(a2,b2)
            print("Площадь равна = ",Striangle2.area())
        if a == 3:
            print("Это нахождение площади квадрата  ")
            a3 = float(input("Введите сторону  "))
            class Hexagon():
                def __init__(self, a3):
                    self.a3 = a3
                def area(self):
                    return self.a3**2
            Shexagon = Hexagon(a3)
            print("Площадь равна = ",Shexagon.area())
        elif a == 4:
            print("Это нахождение площади ромба")
            a4 = float(input("Введите 1 диагональ  "))
            b4 = float(input("Введите 2 диагональ  "))
            class Romb():
                def  __init__(self, a4, b4):
                    self.a4 = a4
                    self.b4 = b4
                def area(self):
                    return (self.a4*self.b4)/2
            Sromb = Romb(a4,b4)
            print("Площадь равна = ",Sromb.area())

Drive link to the code

First, note that finding the area of an isosceles triangle is the same formula as finding the area of right triangle.  Both are 1/2 * base * height.

You are mixing object oriented classes and imperative programming.  This usually happens as you are first learning about classes.  Here is are two alternate structures to your code:

First option, you can ignore classes.   The only commonality between a triangle and an octagon is that both have an area() method.   There is no common data: a triangle has no radius and an octagon has nothing but a radius.  You can make functions like triangle_area(base, height), octagon_radius(radius).  Both return areas.  Just doing this would simplify your code.

Second option, you use classes anyway.  You might do this if you expect more commonality in the near future.  For example, if this is becoming a drawing program then a Figure might be a class that "has a x,y center, current color, a move command, a draw command and an area function."  This code would have code common to all figures in class Figure and would have empty (or virtual) routines that are 'parts any subclass needs to implement to considered a Figure'.

Here is some a code outline to ponder:

 class Figure:
     def area(self):  # return the area of this figure
         raise NotImplementedExeception('subclass must have an area() function')

 class Triangle(Figure):   # inheritence is the promise to follow the superclass API
     def __init__(self, base, height):
         super().__init__()   # this is boilerplate
         self.base, self.height = base, height

     def area(self):
         return self.base * self.height / 2

     @classmethod  # used for custom constructors
     def ask_right(cls):
        print("Это нахождение площади прямоугольного треугольника")
        a1 = float(input("Введите первый катет  "))
        b1 = float(input("Введите второй катет  "))
        return cls(a1, a2)   # return a new triangle

     @classmethod
     def ask_isosceles():
        ...

def main():
        print("Для выхода нажмите Enter")
        a = int(input("1. Площадь прямоугольного тр. \n2. Площадь равнобедренного тр. \n3. Площадь квадрата \n4. Площадь ромба  "))
        if a == 1:
            fig = Triangle.ask_right()
        elif a == 2:
            fig = Triangle.ask_isosceles()
        elif a == 3:
            fig = Octagon.ask()

        print f"Area is {fig.area()}")

In summary, using classes to organize your code makes more sense if you are doing more than just one item.   The real power comes from being able to write functions like:

def redraw(list_of_figures):
    clear_screen()
    for fig in list_of_figures:
        fig.draw()   # which might be Circle.draw(), Triangle.draw() or another subclass

and adding a new subclass, say Pentagon(Figure), without having to change the redraw() function.

Classes can be hard to get your head around; that is normal.

Keep hacking!  Keep notes.


Emulating __uint128_t I developed a lot of code locally with __uint128_t only to find out that it is not available on the target platform.

I am now trying to write my own struct that emulates the operators of __uint128_t I need: =, ~, ^, &, |, ^=, &=, |=

This is the code I have now, which will be compiled with a C++11 compiler:

struct board_t {
    uint64_t hi;
    uint64_t low;

    board_t operator~() const {
        return {~hi, ~low};
    }

    board_t operator|(const board_t & rhs) const {
        return {hi | rhs.hi, low | rhs.low};
    }

    board_t& operator|=(const board_t & rhs) {
        hi |= rhs.hi;
        low |= rhs.low;

        return *this;
    }

    // Complete equivalent functions for &, ^, &= ^=
};

Will anything behave unexpectedly with this code?
Do I need to create a constructor? Is the usage of the {} struct initializer good form?
Do I need to override the assignment operator?
Are there other ways to improve this code, style, correctness and speed?

Will anything behave unexpectedly with this code?

For that to answer we would need the expectations. Most users will expect a number type to behave like the ints do.

What does that mean in your example?

your type should be usable in an if(number) (this also includes if(!number))
your type should be comparable with other int types, nobody wants to write number == board_t{0, 3} instead of number == 3  (this probably means you should write an implicit conversion from int to board_t

Surely there are other things you need to mimic from the ints.

Do I need to create a constructor? Is the usage of the {} struct initializer good form?

Given the fact that you should provide something like board_t(int) you will need to provide the other constructors as well.

Internal functions (that know about the internal format of board_t might use the "struct initialize" but in general I would avoid it as it introduces dependencies on the inner layout.

Do I need to override the assignment operator?

No, the default generated one will simply call the assignments on the members as well which is fine.

Are there other ways to improve this code, style, correctness and speed?

Yes:

why is the type called board_t?
if the user should be able to supply values you could generate an own literal for this purpose
define binary operators like operator+ outside the class to get "more symmetric" code (this makes both operands eligible for automatic conversions instead of only the right hand side)
the previous point may also be applied for unary operators that don't change the internal representation of the object (like operator~())

All in all your design should be:

If the platform supports uint128 the type should only be an alias of it and if it does not support this type, then your class should behave as much as uint128 as possible (so on platforms that support it your code should do the same for both "implementations").


Removing NULL / empty fields just wanted to check with you could this be done better:

awk -F"\t" '{
    for (i = 1; i <= NF; i++) {
        if ($i != "NULL") {
            printf("%s%s", $i, FS);
        }
    }

    printf("\n");
}' file1

The goal is to print only non-NULL fields. For example:

echo "TestRecord001 NULL    NULL    Age 29  NULL    NULL    Name    John" | awk -F"\t" '{
    for (i = 1; i <= NF; i++) {
        if ($i != "NULL") {
            printf("%s%s", $i, FS);
        }
    }

    printf("\n");
}'

will print out: TestRecord001  Age 29  Name    John

The same behaviour can be achieved using sed as follows:

echo -e 'TestRecord001\tNULL\tNULL\tAge\t29\tNULL\tNULL\tName\tJohn' | sed '
s/$/\t/;
s/\(\tNULL\)\+\t/\t/g;
s/^NULL\t//';

Explanation:

sed s/SearchPattern/Replacement/g. Here s indicates that replacement operation is to be done. Strings matching SearchPattern will be replaced by Replacement. g indicates that the operation will have to be performed on every match and not just on the first occurrence in a line.

s/$/\t/ adds a tab to the end of each line. [$ matches the end of a line]
\(\tNULL\)\+\t matches a string of the form \tNULL\tNULL...NULL\t. This is replaced with \t.
After this the only remaining NULL is the one at the beginning of a line (without \t to its left). This is matched by ^NULL\t and replaced with the empty string. [^ matches the beginning of a line]


Changing filename and copying in bash I wrote a script which changes names of last downloaded songs and copy them to music directory. Can it be written better?

    #!/bin/bash
IFS=$'\n'
for file in $(find "/home/stanek/Downloads/" -iname "*.mp3" -mtime -7 | sort)
do
    nowy=$(echo $file \
    | sed "s/_/ /g" \
    | sed "s/\s*(*[A-z0-9.]*.pl)*//g" \
#leave brackets and text if it is remix
    | sed "s/\(\s*(\s\?[A-z0-9 ]*remix)\)\|\s*(\s\?[a-z0-9. ]*)/\1/gi")

    if [ "$nowy" != "$file" ]
    then
        echo "ZMIANA NAZWY DLA:"
        echo $nowy
        mv $file $nowy
    fi
done

for file in $(find "/home/stanek/Downloads/" -iname "*.mp3" -mtime -7 | sort)
do
    echo "I will copy: $file" | sed "s/\/home\/stanek\/Downloads\///g"
done
read -p "Is it ok? (y)" -n 1 -r
echo
if [[ ! $REPLY =~ ^[Yy]$ ]]
then
    exit 1
fi

i=0

for file in $(find "/home/stanek/Downloads/" -iname "*.mp3" -mtime -7 | sort)
do
    paste=$(echo $file | sed "s/\/home\/stanek\/Downloads\///g")
    cp $file "/home/stanek/Music/$paste"
    i=$((i+1))
done
echo "Copied $i files"

"/home/stanek/Downloads/" is used often, consider putting that as a script variable, if it's not already present as an environment variable provided by your shell user profile? Let's assume USER_DOWNLOADS="/home/stanek/Downloads", MUSIC_DIR="/home/stanek/Music" for the suggestions below.

You are invoking find three times, the problem being that new files created during each invocation will be missed by the subsequent one. This may be less of a problem between the first two, but will be for the second and third loops that are separated by a user prompt.

The other question is ask is whether your renaming of files should be done as part of the copying (i.e. later) process, or do you always want to rename the files regardless of whether the copying is done.

If it's the former, then you should only construct the new name within the final loop, for example:

# not *exactly* sanitized for files with spaces, just used as illustration only!
files_to_copy=$(find $USER_DOWNLOADS -iname "*.mp3" -mtime -7 | sort)
echo "Files will be copied:"
for i in $files_to_copy; do basename $i; done
# prompt user
for i in $files_to_copy; do
    new_name=$(basename $i | sed -e 's/.../.../;...');
    cp $i $MUSIC_DIR/$new_name
done

You can string sed commands together: sed -e 's/.../.../;...' as shown above. basename will do the equivalent of your sed "s/\/home\/stanek\/Downloads\///g".

If it's the latter, i.e. same as what you have presented here, then you can leave the first loop aside, but build up your list of files to be copied first:

files_to_copy= # ...
# ...
# prompt user
cp $files_to_copy $MUSIC_DIR
echo "$(awk -F' ' '{print NF}' <<< $files_to_copy) file(s) copied."

To get past filenames with spaces in them, files_to_copy should be constructed as a variable array.


Binary file Handling - Add,modify and display records I have written a basic binary file handling program and have used goto, so I want to know how to improve this program by replacing goto with another function

#include<fstream>
#include<process>
#include<string>
#include<stdio>
#include<conio>

using namespace std;

int opt;
class housing
{
  int hno,income;
  char name[20],type[20];
  public:
  void assign()
  {
   if(income<15000)
   strcpy(type,"LIG");
   else if(income>=15000)
   strcpy(type,"MIG");
   else if(income>=25000)
   strcpy(type,"HIG");
  }
  void input()
  {
   cout<<"\n Enter House Number: ";
   cin>>hno;
   cout<<"\n House Name: ";
   gets(name);
   cout<<"\n Annual Income: ";
   cin>>income;
   assign();
  }
   void output()
  {
   cout<<"House Number: "<<hno<<"\n"<<"House Name: "<<name<<"\n"<<"Annual                                           Income: "<<income<<"\n"<<"Type: "<<type;
  }
  int retno()
  {
  return hno;
  }
};

void main()
{
 housing h,h1;
 fstream f;
 int hono;
 menu:
 cout<<"\n 1:Add a Record"<<'\t'<<"\n 2:Modify"<<'\t'<<"\n 3:Display All  Records"<<'\t'<<"\n 4:Exit"<<endl;
 cin>>opt;
 if(opt==1)
 {
  char ch='y';
  f.open("hous.dat",ios::out|ios::binary|ios::app);
  while(ch=='y')
  {
   cout<<"\n Enter Details: ";
   h.input();
   f.write((char*)&h,sizeof(h));
   cout<<"\n Want to Enter More? y/n: "<<endl;
   cin>>ch;
  }
 f.close();
 }
 if(opt==2)
 {
  cout<<"\n Enter House No of Record to be modified: ";
  cin>>hono;
  f.open("hous.dat",ios::in|ios::out|ios::binary|ios::ate);
  f.seekg(0);
  while(f.read((char*)&h,sizeof(h)))
  {
   if(h.retno()==hono)
   {
    cout<<"\n New Value: ";
    h1.input();
    f.seekp(-sizeof(h),ios::cur);
    f.write((char*)&h1,sizeof(h1));
   }
  }
 f.close();
 }
 if(opt==3)
 {
  f.open("hous.dat",ios::in|ios::binary);
  f.seekg(0);
  while(f.read((char*)&h,sizeof(h)))
  h.output();
  f.close();
 }
 if(opt==4)
 exit(0);
 cout<<"\nPress ... ";
 getch();
 goto menu;

}

Here are some things that may help you improve your code.

Use the required #includes

The code uses strcpy which means that it should #include <cstring>.  It also uses std::cout which means it should #include <iostream>.  Always use the required include files per the standards specification and not just the ones that seem sufficient to make it compile on your machine.

Isolate platform-specific code

In this code, there are several things that are DOS/Windows only including #include <conio> and the getch() function within that, and also #include <process> which I can only guess is a C++ version of the standard process.h.  Your code runs successfully on Linux if I supply those missing functions, but it would be nice if there were an #ifdef WINDOWS already in the code so that one could recompile without having to alter the source code.

Don't use gets

Using gets is not good practice because it can lead to buffer overruns.  It has been removed from the C11 standard and marked "obsolete" in POSIX 2008.  Similarly, it should not be used in C++ program.  Instead use fgets or better, declare name to be a std::string and use std::cin >> name;

Follow standard declaration of main

Your compiler may allow you to declare void main() but that construction is technically only valid for what are called "non-hosted" environments, e.g. one in which you are not running Windows.  For that reason, you should use the standard int main() instead.

Whitespace improves readability

Allofyourwordsarecrammedtogether.  Inserting some spaces in your lines will make them much easier to read.  For example, instead of this:

while(f.read((char*)&h,sizeof(h)))

Use this:

while (f.read((char *)&h, sizeof(h)))

Fix your formatting

The formatting of this code is absolutely diabolical.  The code executed within above-mentioned while loop, for example, is at the same indent level as the while itself. Choose a standard format and use it consistently. Being consistent helps others read and understand your code.

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid.  For this program, I'd advocate removing it and simply using the std:: prefix where needed.

Make better use of C++ objects

The code is already using a housing class, but doesn't make very good use of that class.  For instance, all of the actual binary input and output is done inside main while it could instead be relegated to the housing class.  Also, the file is simply a collection of housing objects.  Why not represent that in a std::vector<housing> or similar?

Create a function rather than repeating code

In several places in the code, the file is opened, read, a housing record read and then the file is closed again.  It would make much more sense and make the code easier to read, understand and maintain, if that functionality were instead encapsulated in a function; probably most appropriately a member function of a house collection object.

Use a switch instead of multiple if or if ...else chains

The logic is much easier to see if a switch statement is used instead of a long if...else chain.  The default case can then be used solely for error cases.

Use longer, more meaningful names

Names like h and f are not very descriptive.  Instead, you might use house and datafile.

Avoid the use of global variables

I see that opt is declared as global variables rather than as local variables. It's generally better to explicitly pass variables your function will need rather than using the vague implicit linkage of a global variable. In this case, there is really no reason that opt couldn't be local to main.

Eliminate the goto

Instead of the goto, use while (!done) and set done to true when the user indicates a desire to exit the program.

Fix the bug

The assign() member function is written like this:

void assign {
    if (income < 15000)
        strcpy(type, "LIG");
    else if (income >= 15000)
        strcpy(type, "MIG");
    else if (income >= 25000)
        strcpy(type, "HIG");
}

However, I think you'll find that HIG will never be assigned no matter how high the reported income.  If the user enters a value of 999999, it will be assigned "MIG".

Eliminate "magic numbers"

In a number of cases, the code uses "magic numbers" such as 20 and 15000 that have no obvious meaning.  These would be better as named constants.  There is a similar argument to be made about the file name "hous.dat" -- this should be made a named constant instead of repeating it three times within the code.


Count frequency of words in a string returning list of unique words sorted by frequency in descending order I have the following solution but I am trying to optimize it to run in O(n) time, any thoughts?

I first split the string removing any non-alphanumeric characters, then split the words using spaces and then sort the new string. The rest is just looping to get the duplicate words and its count which I sort one last time to have the most frequently used word on top of the list.

This function also takes care of empty spaces, and limits the numofwords parameter to the total number of words, to avoid any unexpected results.

function getFrequency(mystring, numofwords) {
    mystring = mystring.replace(/[\.,-\/#!$%\^&\*;:{}=\-_`~()]/g,"");
    var words = mystring.split(' '),
    sortedWords = words.sort(),
    uniqueWords = [],
    d = {},
    wordcount = 1,
    result = [];

    for (var i=0; i<sortedWords.length; i++) {

        var currentword = sortedWords[i];

        if(sortedWords[i+1] === currentword) {
            wordcount++;
        }

        if(!d[currentword]) {
            d[currentword] = true;
            uniqueWords.push({word:currentword, count:wordcount});
        }
    }

uniqueWords = uniqueWords.slice(0,numofwords).sort(function (a,b) { return b.count-a.count;});

for(i=0; i<uniqueWords.length; i++) {

    result.push(uniqueWords[i].word);
}

return result.toString();
}

From a once over:

It seems your code is giving the words with fewest occurrences, consider switching b.count and a.count
I am not vehemently against regexes, but if you use them, I would advise 2 lines of comments, one with the intent of the regex, and one with an example of from -> to
Naming, numofwords -> should be numOfWords or since you only use it once you might as well go for numberOfWords and personally I called it cutOff
Not sure why you named the string myString, I would just call it string or s
It seems that sorting the words in advance will require more code to process the words and it is not guaranteed to be faster, I would not do that
From a data structure perspective, I would go for an object, where each word is a property of the object, and the value is the count, this way you can skip the build
If you are going to sort and slice, then perhaps you should FIRST sort and SECOND slice ;)

This would be my approach:

function getFrequency2(string, cutOff) {
  var cleanString = string.replace(/[\.,-\/#!$%\^&\*;:{}=\-_`~()]/g,""),
      words = cleanString.split(' '),
      frequencies = {},
      word, frequency, i;

  for( i=0; i<words.length; i++ ) {
    word = words[i];
    frequencies[word] = frequencies[word] || 0;
    frequencies[word]++;
  }

  words = Object.keys( frequencies );

  return words.sort(function (a,b) { return frequencies[b] -frequencies[a];}).slice(0,cutOff).toString();
}

document.write( getFrequency2( "1 2 1 2 3 1 2 3 1 1 4 test", 3  ) );


SQL query that flattens many-to-one relationships I have a database with these tables, where each mapping table represents a "many-to-one" relationship between the table on the right and the table on the left (click to enlarge):

I am selecting a single item from product and then adding on all of the other attributes through joins. Here is my current SQL:

SELECT
    p.id,
    p.name,
    p.price,
    p.description,
    GROUP_CONCAT(DISTINCT c.name SEPARATOR ",") AS categories,
    GROUP_CONCAT(og.options SEPARATOR ";") AS options
FROM
    product p
    LEFT JOIN product_category pc ON p.id = pc.product_id
    LEFT JOIN category c ON c.id = pc.category_id
    LEFT JOIN product_optiongroup pog ON p.id = pog.product_id
    LEFT JOIN (
        SELECT
            og.id AS id,
            CONCAT(
                og.name,
                ":",
                GROUP_CONCAT(oi.value)
            ) AS options
        FROM
            optiongroup og
            LEFT JOIN optiongroup_optionitem ogoi ON og.id = ogoi.optiongroup_id
            LEFT JOIN optionitem oi ON oi.id = ogoi.optionitem_id
        GROUP BY og.id
    ) AS og ON og.id = pog.optiongroup_id
WHERE
    p.id = 1 -- This is the input

My desired output is something like:

id    name    price    description    categories     options
-------------------------------------------------------------------------------------------
1     Big Hat 10.00    A good hat.    Hats,Clothing  Color:Black,White;Style:Baseball,Sun

My concern is, would this be a terrible pain to maintain? Are there simpler solutions? Also, does it need to be optimized & how? I don't have much experience with SQL. The output format isn't set in stone; I'm working solo.

Don't LEFT JOIN unless you really need to
In a comment I asked:

Is there a specific reason you are using LEFT JOIN so much? Do you expect products to not have references in those tables a lot? LEFT JOIN are quite expensive and should be used only when needed – Phrancis

To which you responded:

I don't expect it to happen a lot, but I do expect it to happen. In particular, there are products with no options. But there aren't any products with no categories. – 4castle

That being said, you probably ought to build into your script some checks for that. In plain English, something like this might make sense. You can use if/else blocks to make it work.

Check with only a left join on your options. This should cover a large amount of cases, based on your response.

Should that return no rows for an existing product, then try with more left joins to see if something else might be missing.

Should that still return no rows, or invalid data, throw an error.

Aliases
There are a few points from your code that I think can be improved. For instance FROM product p is not a good alias (consider the other aliases as well), as aliases should at least give a hint as to what they are referring to, and p really doesn't. FROM product AS prod would be better in this case.
Also notice the addition of AS keyword which is good practice. It's less of a big deal with table aliases, but it can cause issues in particular with column aliases if it is missing, so it's really best to just include it.


Print all binary strings of length n I have completed my homework with directions as follows:

  Declare and implement a class named Binary. This class will have a
  method named printB(int n) that prints all binary strings of length n.
  For n = 3, it will print

000
001
010
011
100
101
110
111

  in this order.

Is there any way to make this shorter or more efficient?

import java.util.Scanner;
class Binary
{

    String B;
    int temp;

    void printB(int n)
    {
        for(int i = 0; i < Math.pow(2,n); i++)
        {
            B = "";
            int temp = i;
            for (int j = 0; j < n; j++)
            {
                if (temp%2 == 1)
                    B = '1'+B;
                else
                    B = '0'+B;
                    temp = temp/2;
            }
            System.out.println(B);
         }
    }
}

class Runner
{
    public static void main(String [] args)
    {
        Scanner in = new Scanner(System.in);

        System.out.print("Enter n:");
        int n = in.nextInt();

        Binary myB = new Binary();

        myB.printB(n);
    }
}

Is there any way to make this shorter or more efficient?

Yes, we can do some things.

for(int i = 0; i < Math.pow(2,n); i++)

You calculate the pow for every iteration. The pow call takes some time (compared to basic things like multiplication or addition), you should do it only once.

B = "";
B = '1'+B;
B = '0'+B;

You will create a lot of string copies for every string concatenation. This costs a lot of perfomance. In general, you should use a StringBuilder.
In this special case, we could even use a char array.

for(int i = 0; i < Math.pow(2,n); i++)
if (temp%2 == 1)
temp = temp/2;

You could use bit manipulations. But depending on the jvm or hotspot compiler, this will be done anyway.

void printB(int n)
        B = "";
        int temp = i;

Overall point: avoid abbreviations.

All together, it could be like this:

void printAllBinaryUpToLength(final int length) {
    if (length >= 63)
        throw new IllegalArgumentException("Current implementation supports only a length < 63. Given: " + length);
    final long max = 1 << length;
    for (long i = 0; i < max; i++) {
        long currentNumber = i;
        final char[] buffer = new char[length];
        int bufferPosition = buffer.length;
        while (bufferPosition > 0) {
            buffer[--bufferPosition] = (char) (48 + (currentNumber & 1));
            currentNumber >>>= 1;
        }
        System.out.println(buffer);
    }
}

Calculates (without printing, printing takes the great majority of every loop) results up to 25 (2^25 = ~33*10^6) in less than a second. Should be enough.


Code structure for a login modal dialog. I'm using twitter mobile and I have a login-dialog.  login-Dialog is a modal dialog.  To call it the user calls loginDialog.show();

I'm not all that comfortable with the coding style I've adopted (i'm very much self schooled in JavaScript) and I'd be interested to hear peoples opinions on the code structure and process handling.  For example I always seem to use expressive functions instead of function declarations.  (some of the nitty gritty is ommitted for focus).

var loginDialog = (function($)
{
    /**
     * Essentially the init function
     */
    var show = function()
    {
        // This actually shows the dialog
        $("#login-dialog").modal({backdrop: "static", keyboard: false});
        applyBindings();
    }

    var hide = function()
    {
        $("#login-dialog").modal("hide");
    }

    var applyBindings = function()
    {
        $("#login-dialog .btn-primary").click(onLoginClick);
    };

    var onLoginClick = function()
    {
       // validate form, attempt login...
       // logginIn is a jQuery Deferred.
    var loggingIn = myApi.login(username, password);
    loggingIn.done( handleResponse );
    loggingIn.fail( handleError );
    };

    var validate = function(username, password)
    {
       // simple form validation
    };

    var handleResponse = function(json)
    {
            // check json psuedo code
    if (json != "Ok")
    {
        handleError();
    }
    else
    {

        handleSuccess();
    }
    };

    var handleError = function(msg)
    {
        msg = msg || "There was a problem communicating with the server";

        var tmpl = _.template($("#alert-tmpl").html(), { head: "Error", body: msg});

        $("#login-dialog form").append(tmpl);
        $("#login-dialog .btn-primary").removeAttr("disabled");
    };

    var handleSuccess = function()
    {
        hide();
    };

    return {
        show: show,
        hide: hide
    }

})(jQuery);

First off, I'd retrieve all the elements right away, instead of running $(...) again and again:

var loginDialog = (function($) {
  var dialog, form, button, template;

  // Get the elements once, on page load
  $(function () {
    dialog = $("#login-dialog");
    form   = dialog.find("form");
    button = dialog.find(".btn-primary");
    template = $("#alert-tmpl");
  });

  function show() {
    dialog.modal({
      backdrop: "static",
      keyboard: false
    });
  }

  // ... et cetera ...

})(jQuery);

As you can see, I've also put the variable declaration first. If you use expressive functions, you should do the same with all those variables too (which is why I usually prefer to use standard function declarations instead - fewer variables to declare).

(oh, and, yes, I've moved the braces around, but we've covered that in the comments :)

Generally, though, it seems quite fine, and well-structured. There are some functions that could be dropped, since they just call a different function - for instance handleSuccess which is basically an alias for hide. On the other hand it's nicely descriptive to have a handleSuccess function. And if you want to more on success than simply call hide, it's of course nice to have encapsulated.

Lastly, in handleError you .append the error message to the form, so if there are more errors the form will keep growing. Don't know if that's by design, but I imagine you might want to only show one error at a time (and perhaps even remove it on success).


Replacing multiple substrings in a string Here are 2 functions defined that do the exactly same thing: take input a template (in which one wants to replace some substrings) and array of strings values (key value pair to replace, such as [subStrToReplace1,value1,subStrToReplace1,value2,.....]) and returns the replaced String.

In second function I am iterating over words of the templates and searching for the relevant key if exist in a HashMap and then next word. If I want to replace a word with some substring, which I again want to replace with some other key in values, I need to iterate over template twice.

I would like to know which one should I use and why. Any better alternative than these are also welcome.

1st function

public static String populateTemplate1(String template, String... values) {
        String populatedTemplate = template;
        for (int i = 0; i < values.length; i += 2) {
            populatedTemplate = populatedTemplate.replace(values[i], values[i + 1]);
        }

        return populatedTemplate;
    }

2nd function

   public static String populateTemplate2(String template, String... values) {

        HashMap<String, String> map = new HashMap<>();
        for (int i = 0; i < values.length; i += 2) {
            map.put(values[i],values[i+1]);
        }

        StringBuilder regex = new StringBuilder();
        boolean first = true;
        for (String word : map.keySet()) {
            if (first) {
                first = false;
            } else {
                regex.append('|');
            }
            regex.append(Pattern.quote(word));
        }
        Pattern pattern = Pattern.compile(regex.toString());

        int N0OfIterationOverTemplate =2;
        // Pattern allowing to extract only the words
        // Pattern pattern = Pattern.compile("\\w+");
        StringBuilder populatedTemplate=new StringBuilder();;

        String temp_template=template;

        while(N0OfIterationOverTemplate!=0){
        populatedTemplate = new StringBuilder();
        Matcher matcher = pattern.matcher(temp_template);
        int fromIndex = 0;
        while (matcher.find(fromIndex)) {
            // The start index of the current word
            int startIdx = matcher.start();
            if (fromIndex < startIdx) {
                // Add what we have between two words
                populatedTemplate.append(temp_template, fromIndex, startIdx);
            }
            // The current word
            String word = matcher.group();
            // Replace the word by itself or what we have in the map
            // populatedTemplate.append(map.getOrDefault(word, word));

            if (map.get(word) == null) {
                populatedTemplate.append(word);
            }
            else {
                populatedTemplate.append(map.get(word));
            }

            // Start the next find from the end index of the current word
            fromIndex = matcher.end();
        }
        if (fromIndex < temp_template.length()) {
            // Add the remaining sub String
            populatedTemplate.append(temp_template, fromIndex, temp_template.length());
        }

        N0OfIterationOverTemplate--;
        temp_template=populatedTemplate.toString();
    }
        return populatedTemplate.toString();
}

Variable naming

N0OfIterationOverTemplate is not only a mouthful, but anyone will also start to wonder about that weird spelling choice of combining a zero 0 with a capital O together. countIterationOverTemplate is much easier to follow.

null handling

Erm, consider how you want to handle potential null values safely.

Biggest difference

Test case: "template", "a", "b", "a", "c"

In the first method, it immediately replaces the "a"s with "b"s, so the second substitution to "c" is effectively a no-op. You end up with "templbte".

In the second method, there's only one substitution being performed, due to how Map.put(K, V) works. Not only that, it's indeterministic which substitution will happen due to how HashMap.keySet() has no predictable iteration order.

Interfaces over implementations declaration

This is the least of your concern here given the biggest difference above, but you are highly recommended to declare map as a Map instead of HashMap because where it's used, it does not need to be known about its actual Map implementation.


UNC path exists monitor This is my first attempt at trying to write a "monitor" class to determine if a UNC path is available. I need the monitor to not block the main thread and to also event when the UNC is toggled UP/DOWN so that I can respond to that event.

I'm looking for:

Best practice suggestions
Am I doing this properly? Is there a more efficient way?
Will there be threads left alive? will I eat into the pool until it starves?
Other comments suggestions related to this code or programming context

public class NetworkPathMonitor
{
    public event EventHandler<PathAvailabilityChangedArgs> PathAvailabilityChanged;
    public event EventHandler Started;
    public event EventHandler Stopping;
    public event EventHandler Stopped;
    public event EventHandler CheckingStatus;

    protected virtual void OnStopping(object sender, EventArgs e)
    {
        EventHandler handler = Stopping;
        if (handler != null)
            handler(sender, e);
    }
    protected virtual void OnCheckingStatus(object sender, EventArgs e)
    {
        EventHandler handler = CheckingStatus;
        if (handler != null)
            handler(sender, e);
    }
    protected virtual void OnStopped(object sender, EventArgs e)
    {
        EventHandler handler = Stopped;
        if (handler != null)
            handler(sender, e);
    }
    protected virtual void OnStarted(object sender, EventArgs e)
    {
        EventHandler handler = Started;
        if (handler != null)
            handler(sender, e);
    }
    protected virtual void OnPathAvailabilityChanged(object sender, PathAvailabilityChangedArgs e)
    {
        EventHandler<PathAvailabilityChangedArgs> handler = PathAvailabilityChanged;
        if (handler != null)
            handler(sender, e);
    }

    private PathState CurrentState { get; set; }

    private CancellationTokenSource TokenSource { get; set; }

    private AutoResetEvent IsStopping { get; set; }

    public string PathToMonitor { get; set; }

    public int Interval { get; set; }

    public NetworkPathMonitor(string pathToMonitor, int interval, PathState expectedState = PathState.Up)
    {
        this.CurrentState = expectedState;
        this.Interval = interval;
        this.PathToMonitor = pathToMonitor;
    }

    public void Stop()
    {
        OnStopping(this, EventArgs.Empty);
        TokenSource.Cancel();
    }

    public void Start()
    {

        IsStopping = new AutoResetEvent(false);
        TokenSource = new CancellationTokenSource();

        var t = Task.Factory.StartNew(() =>
        {
            OnStarted(this, EventArgs.Empty);
            TimeSpan waitInterval = TimeSpan.FromMilliseconds(Interval);
            while (!IsStopping.WaitOne(waitInterval))
            {
                if (this.TokenSource.IsCancellationRequested)
                {
                    //Is this how to properly stop the signal?
                    IsStopping.Set();
                    IsStopping.Dispose();
                    OnStopped(this, EventArgs.Empty);
                    break;
                }

                OnCheckingStatus(this, EventArgs.Empty);
                CheckPathStatus();
            }
        }, TokenSource.Token);

    }

    private void CheckPathStatus()
    {
        //this is synchronous. We just have to wait for it to timeout.
        var currentState = Directory.Exists(PathToMonitor) ? PathState.Up : PathState.Down;

        var shouldEvent = currentState != CurrentState;
        CurrentState = currentState;

        if (shouldEvent)
        {
            OnPathAvailabilityChanged(this, new PathAvailabilityChangedArgs() { Path = PathToMonitor, PathState = CurrentState });
        }

    }
}

public enum PathState
{
    Up,
    Down
}
public class PathAvailabilityChangedArgs : EventArgs
{
    public string Path { get; set; }
    public PathState PathState { get; set; }
    public PathAvailabilityChangedArgs() { }
    public PathAvailabilityChangedArgs(string path)
    {
        Path = path;
    }
}

The implementation you have will forever consume one thread from the ThreadPool. That's not the end of the world, but not ideal either. An easy solution would be to use an overload of Task.Factory.StartNew that accepts a TaskCreationOptions argument, and specify TaskCreationOptions.LongRunning. That will let the system handle it appropriately. Another option would be starting a new thread yourself.

In the task delegate (the code inside StartNew()) I would put a global try/catch block. Any exceptions that might be thrown in there will cause your process to be killed. At the very least you'd want to log those somewhere just so you have some idea what's going on.

I think you're going to Dispose() the IsStopping event on the first time through the loop, then try to use it on the second time through, which will probably cause an exception. I would just remove the AutoResetEvent entirely and use CancellationTokenSource instead. You can use TokenSource.Token.WaitHandle.WaitOne() to wait on instead of the AutoResetEvent. This also has the benefit that when Stop() is called, any pending wait will end immediately.

I would make the Interval property a TimeSpan instead of an int. It's just more clear to users what it means - they don't have to wonder whether it's seconds, milliseconds, or what.


Find k-distinct partitions which contain at least two perfect squares Problem Statement:

  A k-distinct-partition of a number \$n\$ is a set of \$k\$ distinct positive integers that add up to \$n\$. For example, the 3-distinct partitions of 10 are

  \$1+2+7\$
  \$1+3+6\$
  \$1+4+5\$
  \$2+3+5\$

  The objective is to count all k-distinct partitions of a number that have at least two perfect squares in the
  elements of the partition.
  Note that 1 is considered a perfect square.

Constraints

  \$k<N<200\$, so that at least one k-distinct partition exists.

Input Format

  The input consists of one line containing of \$N\$ and \$k\$ separated by a comma.

Output

  One number denoting the number of k-distinct partitions of N that have at least two perfect squares in the
  elements of the partition.

Explanation

  Example 1

  Input

  10, 3

  Output

  1

  Explanation: The input asks for 3-distinct-partitions of 10. There are 4 of them (1+2+7, 1+3+6, 1+4+5 and
  2+3+5). Of these, only 1 has at least two perfect squares in the partition (1+4+5).

  Example 2

  Input

  12, 3

  Output

  2

  Explanation
  The input asks for 3-distinct partitions of 12. There are 7 of them (9+2+1, 8+3+1,7+4+1,7+3+2,6+5+1,
  6+4+2, 5+4+3). Of these, two, (9+4+1, 7+4+1) have two perfect squares. Hence, the output is 2.

My code hangs when the numbers are big,  e.g. when N = 150 and k = 30

The code

#include <stdio.h>
#include <math.h>

int a[200];
int count=0;
int n,k;
int i,sum,perf;

void func(int,int);
int perfect(int number);

int main()
{
    scanf("%d,%d",&n,&k);
    func(0,1);
    printf("%d",count);

    return 0;
}

void func(int index,int val){
    sum=0,perf=0;
    if(index==k){
        for(i=0;i<k;i++){
            sum=sum+a[i];
        }
        if(sum==n){
            for(i=0;i<k;i++){
                if(perfect(a[i])){
                    perf++;
                }
            }
            if(perf>=2){
                count++;
            }
        }
    }
    else{
        a[index]=val;
        for(i=a[index];a[index]<n;a[index]++){
            func(index+1,a[index]+1);
        }
    }
}

int perfect(int number)
{
    int iVar;
    float fVar;

    fVar=sqrt((double)number);
    iVar=fVar;

    if(iVar==fVar) return 1;
    else return 0;
}

This is an interesting challenge! Here are some ways you could improve your code.

Globals

None of your variables should be global. It makes it significantly harder to figure out where a variable is changing. Given that you reset sum and perf to 0 every time you enter func(), they should be locally declared within the function. i is only used within func() as well. You should prefer declaring variables as close to their first use as possible to make it easier to find the declaration. For loops, it's often best to declare them in the loop like this:

for (int i = 0 ; i < k; i++) {
    ...
}

This also avoids problems that can occur with re-using loop index variables. Notice that you assign to i at the start of the last loop in func(), but then it is never incremented or checked in that loop. It's entirely superfluous, and confusing to read.

func() should return a count rather than incrementing a global variable. For example you could make a local variable and use it like this:

int func(int index, int val) {
    int numPerfectPartitions = 0;
    if (index == k) {
        ...
        if (sum == n) {
            ...
            if (perf >= 2) {
                numPerfectPartitions++;
            }
        }
    }
    else {
        a[index]=val;
        for (...) {
            numPerfectPartitions += func(index + 1, a [ index ] + 1);
        }
    }
    return numPerfectPartitions;
}

Naming

I feel like your names are too vague. perfect() is OK. I would name it isPerfect() just to be more clear, and since it returns what is essentially a Boolean value.

n and k match the text of the challenge, but are bad variable names. I recommend changing n to desiredSum since that's the target number you're trying to reach. (Or even target.) I would name k numPartitions since that's the number of partitions.

a is probably best called something like addends or summands. This is a case where there's no real-world analog, so the names get a little technical. But a is just too vague.

count is an OK name, but what is it a count of? numFound would be better.

func() tells you literally nothing other than that the function is a function, which anyone who can read C already knows. Since it finds partitions, why not call it findPartitions()?

Performance

I'll admit that I don't know off the top of my head of a significantly better way to do this. I suspect that if you first found the perfect squares between 1 and n, then you could find the partitions more quickly. For example, generate all perfect squares between 1 and n, and then find all combinations of 2 through k of the perfect squares that sum to less than or equal to n. Subtract that sum from n to find a new, smaller sum m, and then generate all combinations of the remaining (non-perfect-square) numbers that sum to m. Since you're summing to a smaller value, you should have fewer permutations to go through.


Use of Async/Await for EventHandlers I have a MVC WinForms application. I am using Dependency Injection with Ninject as the IoC container.

public class SqlObjectExplorerController : ToolController, ISqlObjectExplorerController
{
    private ISqlObjectExplorerView view = null;
    private SqlServerStructureProvider structureProvider;
    private IProgress<IProgressInfo> progress;

    public SqlObjectExplorerController(ISqlObjectExplorerView view)
    {
        if (view == null)
            throw new ArgumentNullException("view");

        this.view = view;
        InitializeEventHandlers();
        ...
    }

    private void InitializeEventHandlers()
    {
        (view as ToolView).Initialize += new EventHandler(async (s, e) => await RefreshObjectExplorerAsync());
        view.OnRefreshObjectExplorerClicked += new EventHandler(async (s, e) => await RefreshObjectExplorerAsync());
        view.OnAddServerInstanceClicked += new EventHandler(async (s, e) => await AddServerInstanceAsync());
        view.OnNewSqlQueryClicked += new EventHandler<NewSqlQueryRequestEventArgs>((s, e) => OpenNewSqlQueryDocument(e));
        view.OnExpandRequested += new EventHandler<BeforeExpandEventArgs>((s, e) => BuildSubStructureForDatabaseNode(e));
    }

    private async Task RefreshObjectExplorerAsync()
    {
        await InitializeObjectExplorerAsync();
        view.InitializeObjectExplorer(ServerCache);
    }

    ... // Lots more code.

My question concerns the use of async/await in setting up the likes of += new EventHandler(async (s, e) => await RefreshObjectExplorerAsync());. There are some tasks I need to do on a background thread, so I am using async/await, my questions are about my understanding:

In doing async (s, e) => await SomeMethodAsync() I am merely setting up an event handler equivalent to private async void SomeMethodAsync(object s, EventArgs e) { ... }. Which I think is fine in this case of the fire-and-forget Task I want to undertake, is it?
I don't think there are any problems in setting this up these async handlers in the ctor of the controller, am I right?

The code seems to work fine, but is there something I am not seeing?

The things to consider when it comes to async event handlers are:

Exceptions thrown for the handler might be rethrown on the UI SynchronizationContext, which usually crashes the application.
After you raise the event, the handlers won't be completed yet. The execution of a handler might be interleaved with the execution of the code after the raising and execution of multiple handlers can also be interleaved with each other.

Assuming those caveats are acceptable for you, code like this should be fine.

Also:

view.OnRefreshObjectExplorerClicked += new EventHandler(async (s, e) => await RefreshObjectExplorerAsync());

You should be able to simplify this to just:

view.OnRefreshObjectExplorerClicked += async (s, e) => await RefreshObjectExplorerAsync();

It seems you never unsubscribe the event handlers, are you sure that's okay?