Simple chat console app I wanted to practice using sockets and multithreading. This is simple code where I start a Server and connect to it via Client and chat between them.

Server class:

public class Server {
    public static void main(String[] args) throws Exception{
        ServerSocket serverSocket = new ServerSocket(4545);
        SocketEnhancer socketEnhancer = new SocketEnhancer(serverSocket.accept());
    }
}

Client class:

public class Client {
    public static void main(String[] args) throws Exception{
        SocketEnhancer socketEnhancer = new SocketEnhancer(new Socket("localhost",4545));
    }
}

SocketEnchancer:

public class SocketEnhancer {

    DataReceiver dataReceiver;
    DataSender dataSender;
    Socket socket;

    public SocketEnhancer(Socket socket){
        this.socket = socket;
        dataReceiver = new DataReceiver(this);
        dataSender = new DataSender(this);
        new Thread(dataSender).start();
        new Thread(dataReceiver).start();
    }

    public Socket getSocket() {
        return socket;
    }
}

DataReceiver:

public class DataReceiver implements Runnable {

    SocketEnhancer socketEnhancer;

    public DataReceiver(SocketEnhancer socketEnhancer) {
        this.socketEnhancer = socketEnhancer;
    }

    @Override
    public void run() {
        try {
            BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(socketEnhancer.getSocket().getInputStream()));
            while (true) {
                String readLine = bufferedReader.readLine();
                if(readLine != null){
                    System.out.println("Data received: " + readLine);
                }
            }
        } catch (IOException e) {
            e.printStackTrace();
        }
    }
}

DataSender:

public class DataSender implements Runnable {

    SocketEnhancer socketEnhancer;
    Scanner scanner = new Scanner(System.in);

    public DataSender(SocketEnhancer socketEnhancer) {
        this.socketEnhancer = socketEnhancer;
    }

    @Override
    public void run() {
        try {
            PrintWriter printWriter = new PrintWriter(socketEnhancer.getSocket().getOutputStream());
            while(true) {
                String readLine = scanner.nextLine();
                printWriter.write(readLine + System.lineSeparator());
                printWriter.flush();
            }
        } catch (IOException e) {
            e.printStackTrace();
        }
    }
}

Any comments and criticism is appreciated.

SocketEnhancer socketEnhancer = new SocketEnhancer(serverSocket.accept());

Currently, this is a not very advanced chat application as you can only have one client. It'll be a 1-1 conversation. It will just be one server and one client talking to each other. You can run the serverSocket.accept() in a loop to allow you to have more clients. The server then needs to loop through all the available clients (so you need some Collection for those) and then send the incoming message to all clients.

public static void main(String[] args) throws Exception{
    ServerSocket serverSocket = new ServerSocket(4545);
    SocketEnhancer socketEnhancer = new SocketEnhancer(serverSocket.accept());
}

You don't need to declare the socketEnhancer variable as you're not using it any more in the method. The compiler should give you an unused variable warning here.

DataReceiver dataReceiver;
DataSender dataSender;
Socket socket;

private final, private final, private final. All of those should be private final.

The same goes for all your fields in other classes.

dataReceiver = new DataReceiver(this);
dataSender = new DataSender(this);

So you create a DataReceiver and a DataSender and give them both the SocketEnhancer reference? Let's see how you use that in the classes then.

new InputStreamReader(socketEnhancer.getSocket().getInputStream())

and

socketEnhancer.getSocket().getOutputStream()

You send the entire socketEnhancer only so that you can use the getSocket method on it once to create the OutputStream. It is much better to give them the InputStream / OutputStream directly. This is what is called Tell, Don't Ask!

When possible, don't give your classes an object they can ask about something, give them what they need directly.

public class DataSender implements Runnable {

    Scanner scanner = new Scanner(System.in);
    ...

I would recommend moving the Scanner to your main method, and letting your main method call a method on your SocketEnhancer object whenever it wants to send a message. The SocketEnhancer can then call a method on dataSender to perform the sending.

Overall, I have to say that your code is quite clean and easy to read. Good job.


Summing up distinct elements in steps (follow up) This is a "follow up" to an answer I gave on this question : Summing up distinct elements in steps
Here are the OP's requirements :

"My current task is to find a score from an array where the highest/lowest scores have been taken away, and if the highest/lowest occur more than once (ONLY if they occur more than once), one of them can be added:
E.g. int[] scores = [4, 8, 6, 4, 8, 5] therefore the final addition will be ∑4,8,6,5=23.
Another condition of the task is that LINQ cannot be used, as well as any of the System.Array methods (you can see by my previously ask questions that has been a bit of a pain for me, since I solved this with LINQ in less than 5 minutes)."

public int CalculateScore(int[] scores)
{
   int lowestValue = int.MaxValue,
   highestValue = int.MinValue,
   ammountOfHighestValue = 1,
   ammountOfLowestValue = 1,
   finalScore = 0;

   foreach (int score in scores)
   {
      finalScore += score;

      if (score < lowestValue)
      {
         lowestValue = score;
         ammountOfLowestValue = 1; //We need to reset the ammount
      }
      else if (score > highestValue)
      {
         highestValue = score;
         ammountOfHighestValue = 1; //We need to reset the ammount
      }
      else if (score == lowestValue)
         ammountOfLowestValue++;
      else if (score == highestValue)
         ammountOfHighestValue++;
   }

   if (ammountOfHighestValue > 1)
      //This way, we keep the highest score once.
      finalScore -= ((ammountOfHighestValue - 1) * highestValue);
   else
      finalScore -= highestValue; //The value is there once, we remove it.

   if (ammountOfLowestValue > 1)
      finalScore -= ((ammountOfLowestValue - 1) * lowestValue); //Same as highest
   else
      finalScore -= lowestValue;

   return finalScore;
}

I'm interested about how can I remove the multiple if/else statements  while keeping a complexity of O(n) and still loop through the array only once.

Bugs

Console.WriteLine(CalculateScore(new[] { 1 } ));
Console.WriteLine(CalculateScore(new[] { 2, 1 } ));
Console.WriteLine(CalculateScore(new[] { 3, 2, 1 } ));

What's the expected output? The question is maybe underspecified for the first case (I would say it's 0), but the other two are clear: 0 and 2.

But we get:

-2147483648
-2147483646
-2147483643

Bonus question: what is the correct result for the array { 1, 1 }? I would say 2, but your program returns 0.


First missing positive with only primitives As soon as I saw the First missing positive question, I decided to write my own method for this before writing my review on the question.

Judge me by the same standards as bazang wants to be judged, i.e. As if this was written for a big company such as Google for an interview.

I wasn't sure how the input 7, 8, 9 should be treated so to make it a bit harder for myself I gave myself the requirement that it should return 10.

@Test
public void test() {
    Assert.assertEquals(3, missingInt(new int[]{ 1, 2, 0 }));
    Assert.assertEquals(4, missingInt(new int[]{ 1, 2, 3 }));
    Assert.assertEquals(2, missingInt(new int[]{ 3, 4, -1, 1 }));
    Assert.assertEquals(10, missingInt(new int[]{ 7, 8, 9 }));

    Assert.assertEquals(5, missingInt(new int[]{ 3, 4, 2, 7, 6, 1 }));
    Assert.assertEquals(5, missingInt(new int[]{ 3, 4, 2, 7, 6, 1, -4 }));
}

public int missingInt(int[] array) {
    boolean[] foundIntegers = new boolean[array.length];
    int smallestPositive = Integer.MAX_VALUE;

    for (int i : array) {
        if (i > 0 && i < smallestPositive)
            smallestPositive = i;
    }

    for (int i : array) {
        if (i < smallestPositive)
            continue;

        int index = i - smallestPositive;
        if (index < foundIntegers.length)
            foundIntegers[index] = true;
    }

    for (int i = 0; i < foundIntegers.length; i++) {
        if (!foundIntegers[i])
            return i + smallestPositive;
    }
    return foundIntegers.length + smallestPositive;
}

Stupid edge-cases!
While doing some testing for coming up with a better version for the First missing positive question, I added a bunch of more test cases and compared several different methods with each other. Then I found the following problems:
Input [-5] returned 2147483647 but expected 1
Input [-5, -4, -3] returned 2147483647 but expected 1
Input [0, 0, 0, 0] returned 2147483647 but expected 1
Input [] returned 2147483647 but expected 1

To correct these problems, I added some edge-case checking; to check for array where there are no positive integers. As these really are edge-cases, I haven't decided how to handle these. As there technically isn't a clear "first missing positive number". If I would have handled 7 8 9 as return 1 then I would have returned 1 for these edge cases also, but I feel that wouldn't be expected behavior from this algorithm so therefore I decided to throw an exception.
The new code is:
public int simonNew(int[] array) {
    boolean[] foundIntegers = new boolean[array.length];
    int smallestPositive = Integer.MAX_VALUE;

    for (int i : array) {
        if (i > 0 && i < smallestPositive)
            smallestPositive = i;
    }
    if (smallestPositive == Integer.MAX_VALUE)
        throw new IllegalArgumentException("Array must not be null and must contain at least one positive integer");

    for (int i : array) {
        if (i < smallestPositive)
            continue;

        int index = i - smallestPositive;
        if (index < foundIntegers.length)
            foundIntegers[index] = true;
    }

    for (int i = 0; i < foundIntegers.length; i++) {
        if (!foundIntegers[i])
            return i + smallestPositive;
    }
    return foundIntegers.length + smallestPositive;
}


Convert numbers in a string array (#3, #1, #2, #4) to range (#1:#4) Updated question: Compress numbers in a string array (#3, #1, #2, #4) to range (#1:#4)

This function takes a string array,

{"foo", "#123", "#124", "bar", "#125", "#126"}

makes a new array with the numbers converted to a range:

{"foo", "#123:126", "bar"}

and returns:

"foo,#123:126,bar"

Note it doesn't and shouldn't change {"#1", "#2"} to {"#1:#2"}

static void Main(string[] args)
{
    var ids = new string[] { "foo", "#12", "#14", "#17", "#13", "#18", "#bar", "#19", "#20", "#25", "#26", "#38", "#39", "baz", "#12", "#13", "#14" };
    Console.WriteLine("foo,#12:#14,#17:#20,#bar,#25,#26,#38,#39,baz,#12:#14");
    Console.WriteLine(Convert(ids));
    Console.Read();
}
static string Convert(IEnumerable<string> arr)
{
    var IDs = arr.ToList();
    var result = new List<string>();
    for (var index = 0; index < IDs.Count; index++)
    {
        var i = IDs[index];
        if (i.StartsWith("#"))
        {
            int fromInt;
            if (int.TryParse(i.Substring(1), out fromInt))
            {
                var plus1 = $"#{fromInt + 1}";
                var plus2 = $"#{fromInt + 2}";
                if (IDs.Contains(plus1) && IDs.Contains(plus2))
                {
                    IDs.Remove(plus1);
                    IDs.Remove(plus2);
                    var toInt = fromInt + 2;
                    while (IDs.Contains($"#{toInt + 1}"))
                    {
                        toInt += 1;
                        IDs.Remove($"#{toInt}");
                    }
                    i = $"#{fromInt}:#{toInt}";
                }
            }
        }
        result.Add(i);
    }
    return string.Join(",", result);
}

In order to improve readability I'd say to not go too deep with the indentation. For example:

            if (IDs.Contains(plus1) && IDs.Contains(plus2))
            {
                IDs.Remove(plus1);
                IDs.Remove(plus2);
                var toInt = fromInt + 2;
                while (IDs.Contains($"#{toInt + 1}"))
                {
                    toInt += 1;
                    IDs.Remove($"#{toInt}");
                }
                i = $"#{fromInt}:#{toInt}";
            }

can be transformed in:

if (!(IDs.Contains(plus1) && IDs.Contains(plus2)))
{
    continue;
}
IDs.Remove(plus1);
IDs.Remove(plus2);
var toInt = fromInt + 2;
while (IDs.Contains($"#{toInt + 1}"))
{
    toInt += 1;
    IDs.Remove($"#{toInt}");
}
i = $"#{fromInt}:#{toInt}";

The same can be done with the conditions if (int.TryParse(i.Substring(1), out fromInt)) and if (i.StartsWith("#")) in the upper levels.

The name of the string i is a little misleading. i is usually used as an index. I think id would be a proper name.

The resulting code could be transformed from:

var newIDs = new List<string>();
for (var index = 0; index < IDs.Count; index++)
{
    var i = IDs[index];
    if (i.StartsWith("#"))
    {
        int fromInt;
        if (int.TryParse(i.Substring(1), out fromInt))
        {
            var plus1 = $"#{fromInt + 1}";
            var plus2 = $"#{fromInt + 2}";
            if (IDs.Contains(plus1) && IDs.Contains(plus2))
            {
                IDs.Remove(plus1);
                IDs.Remove(plus2);
                var toInt = fromInt + 2;
                while (IDs.Contains($"#{toInt + 1}"))
                {
                    toInt += 1;
                    IDs.Remove($"#{toInt}");
                }
                i = $"#{fromInt}:#{toInt}";
            }
        }
    }
    newIDs.Add(i);
}
return string.Join(",", newIDs);

into the following:

var newIDs = new List<string>();
for (var index = 0; index < IDs.Count; index++)
{
    var id = IDs[index];
    if (!id.StartsWith("#"))
    {
        newIDs.Add(i);
        continue;
    }
    int fromInt;
    if (!int.TryParse(i.Substring(1), out fromInt))
    {
        newIDs.Add(id);
        continue;
    }
    var plus1 = $"#{fromInt + 1}";
    var plus2 = $"#{fromInt + 2}";
    if (!(IDs.Contains(plus1) && IDs.Contains(plus2)))
    {
        newIDs.Add(id);
        continue;
    }
    IDs.Remove(plus1);
    IDs.Remove(plus2);
    var toInt = fromInt + 2;
    while (IDs.Contains($"#{toInt + 1}"))
    {
        toInt += 1;
        IDs.Remove($"#{toInt}");
    }
    i = $"#{fromInt}:#{toInt}";
    newIDs.Add(id);
}
return string.Join(",", newIDs);


A proxy replacement for getters and setters Have you ever written proxy objects, instead of using a setter and a getter method? In that case, I'm interested in your opinion on the following design for a templated proxy:
#include <type_traits>
#include <utility>

template <typename Handle, typename Getter, typename Setter>
class proxy {
public:
    using value_type = decltype(std::declval<Getter>()(std::declval<Handle>()) );
    operator value_type() const { return getter_(handle_); }
    proxy& operator=(const value_type& x) { setter_(handle_, x); return *this; }
    proxy& operator=(const value_type&& x) { setter_(handle_, x); return *this; }
    proxy(Handle handle, const Getter& getter, const Setter& setter) :
    handle_(handle), getter_(getter), setter_(setter) { }
protected:
    const Handle handle_;
    const Getter& getter_;
    const Setter& setter_;
};

// Allows for template argument deduction during "construction" - before C++17
template <typename Handle, typename Getter, typename Setter>
proxy<Handle, Getter, Setter>
make_proxy(const Handle& handle, const Getter& getter, const Setter& setter)
{
    return proxy<Handle, Getter, Setter>(handle, getter, setter);
}

Simple example of use:
int my_getter(int *x) { return *x; }
void my_setter(int *x, int val ) { *x = val; }

class foo {
public:
    auto datum() {
        return make_proxy(&x, my_getter, my_setter);
    }
protected:
    int x { 123 };
};

int main() {
    foo my_foo;
    my_foo.datum() = 456;
    return my_foo.datum();
}

GodBolt
(in this example, the getter and setter aren't really necessary because the "raw" field exists. But think about opqaue operating-system resources, or individual bits in a bit-container etc.)

Other than general comments on the design - I was also thinking about the choice of template parameters. I might be able to drop the Handle type - if I could manager to extract that information from Getter; or alternatively, I could add the value_type as a template parameter - as otherwise it could be confusing to the person seeing an instantiation to understand what type they should actually use with the proxy.
Also, I was wondering whether I should provide comparators (seeing how this class is convertible to value_type's, which we should be able to compare).
Finally, I was also thinking of not keeping the getter_ and setter_ at all, and only instantiating them on use. But I'm worried this will make the class too convoluted to write and/or use.
Note: This needs to be C++11-compatible.

It's always going to be ugly
Getters and setters are never going to be as pretty in C++ as in some other languages that natively support them. Maybe something nice will be possible in a future C++ standard, but certainly not if you are stuck with C++11.
If you are going to use this to proxy member variables of a class, then the best syntax I can come up with is:
class foo {
    int x{123};
    static int getter(foo *self) { return self->x; }
    static void setter(foo *self, int val) { self->x = val; }
public:
    auto datum() { return make_proxy(this, getter, setter); }
};

The above allows the getter and setter access to the whole class, in case it wants to update multiple variables, or if the getter returns some function of multiple member variables. But unless you can reuse getters and setters, there is really not much point to it in my opinion.
The main reason is that either you have to write my_foo.datum() = 456, which has the added parentheses that make it not look like you are setting a regular member variable, or you have to declare datum like so:
class foo {
    ...
public:
    proxy<foo *, decltype(getter), decltype(setter)> datum{this, getter, setter};
};

That will allow you to write my_foo.datum = 456.
The former doesn't even work with C++11, since auto return type deduction doesn't work in that situation. You could add a trailing return type, but it will be ugly and repetetive. The latter has the overhead of storing three pointers for each proxy variable in your class.
It doesn't handle const instances
Your proxy class doesn't work if you create a const instance of a class that uses proxy member variables, for example:
const foo my_foo;
return my_foo.datum();

You could probably create a class const_proxy that only has a getter, and which ensures const is used in the right places, and then overload the proxy member like so:
class foo {
    ...
public:
     auto datum() { return make_proxy(this, getter, setter); }
     auto datum() const { return make_const_proxy(this, getter); }
};

But that adds even more noise.


Python Barcode Generator I was asked to provide some example code for a job interview and didn't have anything to offer, so wrote the following function.
A barcode generator may seem a bit basic, but there's some logic behind the choice:

It's something I recently needed, so not entirely contrived.
It's simple enough to do in one function, so it can be reviewed easily.
It's complex enough that a new programmer would make a mess of it.
It's visual. You can render the result in a browser.
It's useful, so I could make a little gist out of it.

The barcode is in the Interleaved 2 of 5 Format. It's a simple format, and there's a Wikipedia article that covers it well.
Update: A new version of this code has been posted here. The new code includes many of the suggestions made below.
def render(digits):

    '''This function converts its input, a string of decimal digits, into a
    barcode, using the interleaved 2 of 5 format. The input string must not
    contain an odd number of digits. The output is an SVG string.

        import barcode
        svg = barcode.render('0123456789')

    Strings are used to avoid problems with insignificant zeros, and allow
    for iterating over the bits in a byte [for zipping].

    # Wikipedia, ITF Format: http://en.wikipedia.org/wiki/Interleaved_2_of_5
    '''

    # Encode: Convert the digits into an interleaved string of bits, with a
    # start code of four narrow bars [four zeros].

    bits = '0000'
    bytes = {
        '0': '00110', '5': '10100',
        '1': '10001', '6': '01100',
        '2': '01001', '7': '00011',
        '3': '11000', '8': '10010',
        '4': '00101', '9': '01010'
        }

    for i in range(0, len(digits), 2):

        # get the next two digits and convert them to bytes
        black, white = bytes[digits[i]], bytes[digits[i+1]]

        # zip and concat to `bits` 5 pairs of bits from the 2 bytes
        for black, white in zip(black, white): bits += black + white

    # Render: Convert the string of bits into an SVG string and return it.

    svg = '<svg height="50">'
    bar = '<rect x="{0}" y="0" width="{1}" height="50" {2}></rect>'
    pos = 5 # the `x` attribute of the next bar [position from the left]

    for i, bit in enumerate(bits):

        width = int(bit) * 2 + 2
        color = '0,0,0' if (i % 2 == 0) else '255,255,255'
        style = 'style="fill:rgb({0})"'.format(color)

        svg += bar.format(pos, width, style)
        pos += width

    return svg + '</svg>'

Note: This works in both Python 2 and Python 3

The code generally looks pretty good, but I think there are some things you could fix up here.

Don't forget the stop code

You have the start code of 0000 but you've forgotten the stop code which is 100.  When you calculate, you could just tack it onto the end with bits += '100'

Use SVG <def>s

Since the purpose is to create an SVG barcode, it make sense to tighten the resulting SVG.  You may already know how to use a <def> in SVG and it definitely makes things a little easier to understand here.  The way your code is currently structures, it creates four different styles of <rect> which are all combinations of narrow/wide and black/white.  You could predefine each of those shapes and then simply intantiate them within the body of the svg code.  That would make the definition for svg and bar like this:

svg = '''<svg height="50"><defs>
<g id="b0"><rect x="0" y="0" width="2" height="50"/></g>
<g id="b1"><rect x="0" y="0" width="4" height="50"/></g>
</defs>'''
bar = '<use xlink:href="#b{0}" x="{1}" y="0" {2}/>'

Using it would then be

svg += bar.format(bit, pos, style)

An improvement would be to create both bars and spaces like this:

<g id="b0"><rect x="0" y="0" width="2" height="50" style="fill:rgb(0,0,0)"/></g>
<g id="b1"><rect x="0" y="0" width="4" height="50" style="fill:rgb(0,0,0)"/></g>
<g id="s0"><rect x="0" y="0" width="2" height="50" style="fill:rgb(255,255,255)"/></g>
<g id="s1"><rect x="0" y="0" width="4" height="50" style="fill:rgb(255,255,255)"/></g>

Then your loop could look then like this:

for i, bit in enumerate(bits):
    width = int(bit) * 2 + 2
    svg += bar.format('bs'[i%2], bit, pos)
    pos += width

Although that may look verbose, it actually is over 2k shorter for a 12 digit barcode.

Think carefully about data representation

Your code currently translates digits into a series of '1' and '0' characters and then translates again into SVG rectangles.  Why not eliminate a step? Your code could just as easily translate them in a single operation.

Use list comprehensions instead of for loops

The use of list comprehensions is almost always faster than a for loop in Python, so we use them when we can.  It also tends to make the code shorter.  So for example, we could change your loop to calculate the string of bar code bits to calculate all the black bits and then all the white bits like this:

black = "".join([bytes[i] for i in digits[0::2]])
white = "".join([bytes[i] for i in digits[1::2]])
# shuffle them together
databits = "".join("".join(i) for i in zip(black,white))
# create the full bar code string with start and stop
bits = "".join(['0000',databits,'100'])

Doing this with join also saves time.  Appending strings with += is very slow in Python.

Prefer xrange to range

When you use range, a whole list object is created, using memory.  With xrange, a generator is created instead populated which can save memory.  For any reasonably sized bar code this won't make much difference here, but it's good practice for Python 2.7.  In Python 3, xrange doesn't exist and range creates a generator, so keep that in mind if you change versions.

Don't draw more than you have to

You're drawing one <rect> for every bar or space, but it's really not necessary.  Instead, you could create one larger rectangle that's the "background" space color and then only draw the black bars.


Query for finding collection entries matching an arbitrary set of URLs Most of my slow queries on my server are due to a single "find" query. I have a collection that represents website pages. I want to be able to look up a page in the collection by URL. The caveat is that a website might have URLs that redirect to it.

I've modeled my schema (with Node.js/Mongoose) like this:

{
        'url':                      { type: String, index: {unique: true, dropDups: true} },
        'redirects':                { type: Array, index: true },
        // ... other stuff
    }

In other words, one entry might have the URL http://codereview.stackexchange.com/ but also have an array of 5 other URLs (such as bit.ly URLs) that redirect to it.

Later, I want to query the collection to see if we have an entry which matches an arbitrary set of URLs. I don't know if the URLs are redirected URLs or what -- I just want to find all matches in my collection that represent these URLs.

So I do this:

// urls is an array of URL strings we want to find...
model.find({$or: [{'url': {$in: urls}}, {'redirects': {$in: urls}}]}).lean().exec( ... );

Unfortunately, I might be looking for around 200 URLs at once, so this query sometimes takes > 1 second.

Is there further optimization I can do, or would it be better to split the query into multiple queries, capping the search-size each time?

I haven't used MongoDB, but I have a few suggestions based on experiences with optimization and other databases.

Index hashes of the URLs and search for those instead. Using a simple MD5 hash would probably speed up searching with the cost of dealing with false positives (unlikely but possible).
Store every URL as a top-level object and add a redirectsTo attribute. This alleviates the need to index or search the redirects collections. While now you need to perform two queries--one for the original URLs and another for the redirected URLs--this could end up being faster if only a small percentage of URLs are redirects.

Here's some psuedocode to clarify what I mean:

all = []
found = model.find({'url': {$in: urls}})...
redirects = []
for each item in found
    if item.redirectsTo
        redirects += item.redirectsTo
    else
        all += item
redirected += model.find({'url': {$in: redirects}})...
for each item in redirected
    all += item

Update your question with the higher-level problem you're trying to solve. I often find that when I'm trying to optimize slow task T to solve problem P, the better approach is to rethink P.


Quicksort implementation using filter() to perform partitioning from operator import ge as greater, lt as lesser

def qsort(L):
    if len(L) <= 1: return L
    pivot   = L[0]
    sublist = lambda op: [*filter(lambda num: op(num, pivot), L[1:])]

    return qsort(sublist(lesser))+ [pivot] + qsort(sublist(greater))

It seems like an overkill to have two lambdas in the sublist definition. How can I refactor the sublist lambda function to not include lambda at all?

I think having one lambda in your definition of sublist is perfectly appropriate, but the use of filter isn't appropriate because you are going to need a list anyway. You aren't using it wrong, there are just better solutions.

Also, as noted in the other answer, you can avoid repeated slicing L by creating a copy of the list on the first run of the function through an optional default argument (see first in the code below).

Finally, summing three lists in your return statement is probably less than optimal. With unpacking in Python 3, you can turn this into a single comprehension which should be better in terms of intermediate object creation.

from operator import ge, lt

def qsort(L, first=True):
    if len(L) <= 1:
        return L

    L = L[:] if first else L
    pivot = L.pop()
    sublist = lambda op: [n for n in L if op(n, pivot)]

    return [*qsort(sublist(lt), False), pivot, *qsort(sublist(ge), False)]


Temperature Converter This is an implementation of a practice problem from cprogramming.com:  Temperature Converter Challenge

The problem:

  In this challenge, write a program that takes in three arguments, a start temperature (in Celsius), an end temperature (in Celsius) and a step size. Print out a table that goes from the start temperature to the end temperature, in steps of the step size; you do not actually need to print the final end temperature if the step size does not exactly match. You should perform input validation: do not accept start temperatures less than a lower limit (which your code should specify as a constant) or higher than an upper limit (which your code should also specify). You should not allow a step size greater than the difference in temperatures. (This exercise was based on a problem from C Programming Language).

The code:

#include <iostream>
#include <string>
#include <sstream>
#include <iomanip>

using namespace std;

const double LowestLimit = 0;
const double HighestLimit = 50000;
const string LowerLimitMessage = "Please enter a lower limit, limit >= 0: ";

struct UserInputs
{
public:
    double LowerLimit = -1;
    double UpperLimit = -1;
    double StepAmount = -1;
};

stringstream GetSingleInput(const string message)
{
    stringstream returnStream;
    string input;
    cout << message;
    cin >> input;
    returnStream << input;
    return returnStream;
}

UserInputs GetUserInputs()
{
    UserInputs parsedInputs;
    auto LowerLimitStream = GetSingleInput(LowerLimitMessage);

    while (!(LowerLimitStream >> parsedInputs.LowerLimit) || parsedInputs.LowerLimit < LowestLimit)
    {
        cout << "Invalid Lower Limit!\n";
        LowerLimitStream = GetSingleInput(LowerLimitMessage);;
    }

    stringstream UpperLimitMessage;
    UpperLimitMessage << "Please enter an upper limit, limit > "
        << parsedInputs.LowerLimit
        << ", limit <= "
        << HighestLimit
        << ": ";

    auto UpperLimitStream = GetSingleInput(UpperLimitMessage.str());

    while (!(UpperLimitStream >> parsedInputs.UpperLimit)
        || parsedInputs.UpperLimit > HighestLimit
        || parsedInputs.UpperLimit <= parsedInputs.LowerLimit)
    {
        cout << "Invalid Upper Limit!\n";
        UpperLimitStream = GetSingleInput(UpperLimitMessage.str());
    }

    stringstream stepMessage;
    stepMessage << "Please enter a step amount , step > "
        << LowestLimit
        << ", step <= "
        << (parsedInputs.UpperLimit - parsedInputs.LowerLimit)
        << ": ";

    auto stepStream = GetSingleInput(stepMessage.str());
    while (!(stepStream >> parsedInputs.StepAmount)
        || parsedInputs.StepAmount <= 0
        || parsedInputs.StepAmount > parsedInputs.UpperLimit - parsedInputs.LowerLimit)
    {
        cout << "Invalid Step Amount!\n";
        stepStream = GetSingleInput(stepMessage.str());
    }

    return parsedInputs;
}

double CelsiusToFarenheit(const double celsius)
{
    return celsius * 33.8;
}

int main()
{
    auto inputs = GetUserInputs();

    cout << "Celsius" << "\t\t\t" << "Farenheit" << "\n";
    cout << "----------" << "\t\t" << "----------" << "\n";
    cout.precision(9);
    while (inputs.LowerLimit <= inputs.UpperLimit)
    {
        cout << fixed << inputs.LowerLimit << "\t\t" << CelsiusToFarenheit(inputs.LowerLimit) << "\n";
        inputs.LowerLimit += inputs.StepAmount;
    }

    return 0;
}

I'm a C# developer who has been self learning C++. As a result I am likely making simple mistakes on structural design which don't feel odd in C#.

Any commentary would be appreciated, especially as to whether or not my method of checking the input is inefficient.

Your conversion from celcius is not correct, since 0°F is not 0°C. Something like \$F = \frac{9}{5} C + 32\$ should work for the conversion.


Prints each letter of a string in different colors I'm practicing for my Technical Interview and I'm worried my code does not meet the bar. What can I do to improve this answer?

Here is the question: https://www.careercup.com/question?id=5739126414901248

Given

colors = ["red", "blue", "green", "yellow"]; and a string

str = "Lorem ipsum dolor sit amet";

write a function that prints each letter of the string in different colors. ex. L is red, o is blue, r is green, e is yellow, m is red, after the space, i should be blue.

My answer:

static void TestPrintColors()
    {
        string[] colors = new string[4] {"red", "blue", "green", "yellow"};
        string str = "Lorem ipsum dolor sit amet";
        PrintColors(colors, str);
    }

    static void PrintColors(string[] colors, string str)
    {
        char log;
        ConsoleColor originalColor = Console.ForegroundColor;
        int colorIndex = 0;
        ConsoleColor currentColor = originalColor;
        for (int i = 0; i < str.Length; i++)
        {
            log = str[i];

            if (log == ' ')
            {
                Console.WriteLine(log);
                continue;
            }

            switch(colors[colorIndex])
            {
                case "red":
                currentColor = ConsoleColor.Red;
                break;

                case "blue":
                currentColor = ConsoleColor.Blue;
                break;

                case "green":
                currentColor = ConsoleColor.Green;
                break;

                case "yellow":
                currentColor = ConsoleColor.Yellow;
                break;

                default:
                currentColor = originalColor;
                break;
            }

            colorIndex++;

            if (colorIndex >= colors.Length)
            {
                colorIndex = 0;
            }

            Console.ForegroundColor = currentColor;

            Console.WriteLine(log);
        }

        Console.ForegroundColor = originalColor;
    }

Don't worry about performance.

You're writing a handful of strings to the console, and it takes milliseconds. When you start writing tens of thousands of strings, and notice it taking seconds, then you can you start to look for optimizations. Until then, the cleanliness of your code is much more important.

Your function is too long.

This is a fifty-line function. It has the responsibilities of iterating through two lists in parallel, skipping spaces, parsing color names, setting the console color, writing to console, and resetting the console color when it's all done. That's a lot! Break it up into smaller functions.

Switch statements are ugly.

I don't mean that they are never appropriate, but ConsoleColor is an enum, and it's possible to parse enums (while ignoring case sensitivity). You should replace this switch statement with a function call.

Don't initialize variables until you need them.

With few exceptions, modern languages are very good about optimizing variable allocation. Putting char log = str[i] inside the loop will not result in additional memory usage, and it will save me (a potential reviewer or maintenance programmer) from having to think about that character before or after the loop.

Other tips...

You say this is practice for an interview, so it could be a good place for you to show off your knowledge of C#. With a little trouble, you could leverage Regular Expressions and LINQ to save you from manually manipulating array indexes. With a little more trouble, you could leverage IDisposable to ensure the original ForegroundColor is restored when all is said and done.

On the other hand, you could also shoot yourself in the foot attempting to do those things. If you don't honestly have in-depth knowledge about C#, it might be best just to aim for code that is as simple as possible. I think the best way to do that is to make small functions with clear names, to show you are thinking about the maintainability and reusability of your code.


Expression tree creation from postfix expression Given a postfix expression, construct an expression tree. Looking code code review, optimizations and best practices.

public class ExpressionTree {

    private final String postfix;
    private TreeNode root;

    /**
     * Takes in a valid postfix expression and later its used to construct the expression tree.
     * The posfix expression, if invalid, leads to invalid results
     *
     * @param postfix   the postfix expression.
     */
    public ExpressionTree(String postfix) {
        if (postfix == null) { throw new NullPointerException("The posfix should not be null"); }
        if (postfix.length() == 0)  { throw new IllegalArgumentException("The postfix should not be empty"); }
        this.postfix = postfix;
    }

    private static class TreeNode {
        TreeNode left;
        char ch;
        TreeNode right;

        TreeNode(TreeNode left, char ch, TreeNode right) {
            this.left = left;
            this.ch = ch;
            this.right = right;
        }
    }

    private boolean isOperator(char c) {
        return c == '+' || c == '-' || c == '*' || c == '/';
    }

    /**
     * Constructs an expression tree, using the postfix expression
     */
    public void createExpressionTree() {
        final Stack<TreeNode> nodes = new Stack<TreeNode>();
        for (int i = 0; i < postfix.length(); i++) {
            char ch  = postfix.charAt(i);
            if (isOperator(ch)) {
               TreeNode rightNode = nodes.pop();
               TreeNode leftNode = nodes.pop();
               nodes.push(new TreeNode(leftNode, ch, rightNode));
            } else {
                nodes.add(new TreeNode(null, ch, null));
            }
        }
        root = nodes.pop();
    }

    /**
     * Returns the prefix notation
     *
     * @return the prefix notation
     */
    public String prefix() {
        if (root == null) {
            throw new NoSuchElementException("The root is empty, the tree has not yet been constructed.");
        }

        final StringBuilder prefix = new StringBuilder();
        preOrder(root, prefix);
        return prefix.toString();
    }

    private void preOrder(TreeNode node, StringBuilder prefix) {
        if (node != null) {
            prefix.append(node.ch);
            preOrder(node.left, prefix);
            preOrder(node.right, prefix);
        }
    }

    /**
     * Returns the infix expression
     *
     * @return  the string of infix.
     */
    public String infix() {
        if (root == null) {
            throw new NoSuchElementException("The root is empty, the tree has not yet been constructed.");
        }
        final StringBuilder infix = new StringBuilder();
        inOrder(root, infix);
        return infix.toString();
    }

    private void inOrder(TreeNode node, StringBuilder infix) {
        if (node != null) {
            inOrder(node.left, infix);
            infix.append(node.ch);
            inOrder(node.right, infix);
        }
    }

    public static void main(String[] args) {
        ExpressionTree expressionTree1 = new ExpressionTree("AB*CD/+");
        expressionTree1.createExpressionTree();
        assertEquals("+*AB/CD", expressionTree1.prefix());
        assertEquals("A*B+C/D", expressionTree1.infix());

        ExpressionTree expressionTree2 = new ExpressionTree("ABC+*D/");
        expressionTree2.createExpressionTree();
        assertEquals("/*A+BCD", expressionTree2.prefix());
        assertEquals("A*B+C/D", expressionTree2.infix());

        ExpressionTree expressionTree3 = new ExpressionTree("ABCD/+*");
        expressionTree3.createExpressionTree();
        assertEquals("*A+B/CD", expressionTree3.prefix());
        assertEquals("A*B+C/D", expressionTree3.infix());
    }
}

Make your structs immutable
TreeNode is a helper structure, with public members, which is fine, but you better make them final, so you know they won't be changed after the object is created.

Choose your validations
Don't throw NullPointerException on your own. It may confuse a future debugger. Either throw an IllegalArgumentException or let the runtime throw the NullPointerException for you.

Choose your comments
Most of your comments do not add much to the methods, and are therefore redundant. Let the names of the method and variables do the work for you. If you feel the name prefix is not clear enough on its own, it is better to rename it (maybe to toPrefixNotation()) and then you can safely remove your comments.

Being nice is better than being strict
Unless there is some requirement restriction, I don't think you need to throw an exception if root is null - why not simply call createExpressionTree() instead of telling the developer he should?
Since there is no meaning for calling createExpressionTree() twice (postfix is final...), you might as well hide it altogether, and call it when needed.

Use the power of String
Use String#indexOf to make isOperator more succinct:

private boolean isOperator(char c) {
    return "+-*/".indexOf(c) != -1
}


Random string generating function Both these functions take in a sentence and generate a random string based of the length of sentence.

Mine, the first one, runs 4 times more slowly than the second one (someone else's code), but I don't know why and I'd like to understand what's happening 'under the hood' that makes it this way. If anything I would've expected numpy to be faster, which makes me think it's something else entirely.

Also, is there a way to make the first function even faster?

import random
import numpy as np

def generate_guess(sentence):
    alphabet = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z', ' ']
    return [np.random.choice(alphabet) for i in range(len(sentence))]

Don't review this one:

def generate_guess(sentence):
  alphabet = 'abcdefghijklmnopqrstuvwxyz '
  res = ''
  for i in range(len(sentence)):
      res = res + alphabet[random.randrange(len(alphabet))]
  return res

np.random.choice takes an additional argument specifying how many elements should be chosen. You can also use the built-in module string to save having to spell out the alphabet:

import numpy as np
import string

ALPHABET = np.array(list(string.ascii_lowercase + ' '))

def generate_guess(sentence):
    return np.random.choice(ALPHABET, size=len(sentence))

I also made the alphabet a constant to avoid having to build it again and again.

Your code was not faster because you did not take advantage of the vector-wise operations of numpy. Neither your starting array, the alphabet, nor your output array were actual numpy arrays.


Time optimization of counting shifts in insertion sort Is there any way of counting how many shifts would it take to count shifts while doing insertion sort?  This is a Hackerrank problem. Here's my solution, but it takes more than 16 seconds.

counts = []
for _ in range(int(input())):
    size = int(input())
    ar = list(map(int, input().split()))
    count = 0
    for i in range(1, len(ar)):
        j = i
        while j >= 1 and ar[j] < ar[j-1]:
            ar[j], ar[j-1] = ar[j-1], ar[j]
            j -= 1
            count+=1
    counts.append(count)
for c in counts: print(c)

  Insertion Sort is a simple sorting technique which was covered in
  previous challenges. Sometimes, arrays may be too large for us to wait
  around for insertion sort to finish. Is there some other way we can
  calculate the number of times Insertion Sort shifts each elements when
  sorting an array?

  If ki is the number of elements over which ith element of the array
  has to shift then total number of shift will be k1 + k2 + … + kN.

  Input: The first line contains the number of test cases T. T test
  cases follow. The first line for each case contains N, the number of
  elements to be sorted. The next line contains N integers
  a[1],a[2]…,a[N].

  Output: Output T lines, containing the required answer for each test
  case.

That's the problem. Is there any way I could optimize my solution?

Insertion sort is an O(n2) algorithm.  There's only so much you can do to optimize it.

This is a bit of trick question.  It doesn't actually ask you to sort the arrays in question; it just asks you to count the number of shifts involved.  So, for each element to be "inserted", just count the number of preceding elements that have a larger value.

Stylistically, your program should be modularized.  Each of the T test cases is independent, so isolate each test case in a function.  You probably don't have to buffer the results, either — just print the result from each test case as soon as you have the answer.


Show byte representation of generic type I am new to Rust. I would like to know any better way of doing this and if it can be done preferably with safe code.

use std::mem;
use std::mem::size_of;

fn show_byte <T> ( element: T) {

    let array: * mut u8 = unsafe{ mem::transmute(&element) };
    for i in (0.. size_of::<T>()) {
         let current = (array as usize + i) as *mut u8;
         print!("{:X}", unsafe { *current });
    }
}

fn main() {
   show_byte(-1i64);
}

I am not aware of any safe method in the standard library to do this, so we need some unsafe code.

In your code, you have two unsafe blocks. However, the first one is unnecessary: you can use as to perform the cast, but you need two consecutive casts, because we can't cast from &T to *mut u8 directly. Actually, you should use *const u8 here instead, since you don't actually mutate the referent. So, instead of mem::transmute, you can write:

let array = &element as *const T as *const u8;

If you really need a *mut u8, then you'd need to define element as mut and write &mut element instead of &element.

However, I would do this differently. I'd write a function to turn a &T into a slice of bytes (&[u8]), which would encapsulate the only unsafe operation of the program. Then, we can use a simple for loop on that slice to print each byte.

use std::mem;
use std::slice;

fn bytes_of<T>(value: &T) -> &[u8] {
    unsafe {
        slice::from_raw_parts(
            value as *const T as *const u8, mem::size_of::<T>())
    }
}

fn show_as_bytes<T>(element: T) {
    for b in bytes_of(&element) {
        print!("{:X}", b);
    }
}

fn main() {
    show_as_bytes(-1i64);
}

Note that the bytes_of function plays a very important role that would be lost if it was inlined (by hand): the lifetime of the returned byte slice is tied to the lifetime of the input parameter. It's as if it was written thus:

fn bytes_of<'a, T>(value: &'a T) -> &'a [u8] {
    // ...
}

If the function's body was inlined, the slice returned by slice::from_raw_parts would be unconstrained, and the compiler wouldn't be able to report an error if the slice accidentally outlived the data it references, causing subtle memory errors at runtime.

Bonus: Here's how I would format your original code (note the changes to whitespace and the removal of parentheses).

use std::mem::size_of;

fn show_byte<T>(element: T) {
    let array: *mut u8 = unsafe { mem::transmute(&element) };
    for i in 0..mem::size_of::<T>() {
         let current = (array as usize + i) as *mut u8;
         print!("{:X}", unsafe { *current });
    }
}

fn main() {
    show_byte(-1i64);
}


Configuration modules I started writing a new framework in C# which can be used for any server development. It contains the basic stuff, such as database, TCP and configuration. I did write the configuration last night and I would love to get some constructive criticism and tips on it.

I know I didn't document the code yet, planning on doing it once I'm 100% happy with it.

First off, we have the BaseConfiguration class. This class is basically the base (superclass) of all configuration modules.

using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using System.IO;

namespace AuroraFramework.Configuration
{
    public abstract class BaseConfiguration
    {
        private readonly string filePath;
        private JObject jsonSettings;

        protected BaseConfiguration(string filePath)
        {
            this.filePath = filePath;
            ReadFile();
        }

        private void ReadFile()
        {
            StreamReader reader = new StreamReader(filePath);

            jsonSettings = JsonConvert.DeserializeObject<JObject>(reader.ReadToEnd());
        }

        protected T Get<T>(string key)
        {
            return jsonSettings[key].Value<T>();
        }
    }
}

What it does is simple. It reads the file you pass down in the constructor, and 'converts' it into a JObject. That JObject is used to get values from. Then the Get function is used to get values from the JObject and return it in the right datatype. The reason I chose for this is to have more control than when I use JsonConvert.DeserializeObject<TYPE>(string).

The whole class is used to it's easy to implement new configuration modules without reading the whole file on your own and without writing much code.

Next, we have the DatabaseConfiguration class, which is simply one of the configuration modules in the framework.

namespace AuroraFramework.Configuration.Types
{
    public class DatabaseConfiguration : BaseConfiguration
    {
        public string Server { get; set; }
        public string Username { get; set; }
        public string Password { get; set; }
        public string Database { get; set; }

        public DatabaseConfiguration()
            : base("db.settings.json")
        {
            Server = Get<string>("server");
            Username = Get<string>("username");
            Password = Get<string>("password");
            Database = Get<string>("database");
        }
    }
}

This simply uses the BaseConfiguration class as superclass. This will let the superclass read the file and convert it to a JObject. The only thing for the configuration module itself to do is call the Get function on every property.

And then, you use DatabaseConfiguration dbConfig = new DatabaseConfiguration() in the framework and pass it down to the classes that need to use the configuration.

Do you really want the properties of DatabaseConfiguration to be settable?

  public string Server { get; set; }

If time is not an issue, I would make the call of each property "lazy":

public string Server => Get<string>("server");


Finding the minimum number of swaps to sort a list I can handle large numbers with this code as well but I need to improve my execution time.

def no_swap(a):
  sorted_a=sorted(a)
  rev_a=list(reversed(a))
  b=a[:]
  n=len(a)
  noswap1=0
  noswap2=0
  for i in range(n):
    if a[i]!=sorted_a[i]:
      noswap1=noswap1+1
      j=binarySearch(sorted_a[i],a)
      a=swaplist(i,j,a)

    if rev_a[i]!=sorted_a[i]:
      noswap2=noswap2+1
      j=binarySearch(sorted_a[i],rev_a)
      b=swaplist(i,j,rev_a)

  return min(noswap1,noswap2)

def swaplist(i,j,arr):
  temp=arr[i]
  arr[i]=arr[j]
  arr[j]=temp
  return arr

def binarySearch(item, alist):
  first = 0
  last = len(alist)-1
  found = False
  while first<=last and not found:
    midpoint = (first + last)
    if alist[midpoint] == item:
      found = True
    else:
      if item < alist[midpoint]:
       last = midpoint-1
      else:
        first = midpoint+1

  return midpoint

Any suggestions are appreciated!

1. It's accidentally quadratic

The line:

midpoint = (first + last)

should be:

midpoint = (first + last) // 2

The consequence of this mistake is that binarySearch takes \$Θ(n)\$ to find the location, instead of the intended \$O(\log n)\$. And binarySearch is called for every item in the list, making the overall runtime \$Θ(n^2)\$ instead of the intended \$O(n \log n)\$.

2. Review of binarySearch

There's no docstring. What does it do? What arguments do I pass? What does it return?
The Python style guide (PEP8) recommends:

  4 spaces per indentation level

and:

  Always surround these binary operators with a single space on either side:  [...] comparisons (<=), [...]

Following PEP8 will make it easier for you to collaborate with other Python programmers.
A local variable found is used to exit the while loop. But Python's break statement also exits a loop, and if you used this you'd avoid the need for the variable.
else: if can be written elif:, saving a level of indentation.
Consider using the built-in bisect module.

3. Alternative approach

The rearrangement of items that happens when you sort a list is known as a permutation. For example, let's take this list of items:

When we sort the list, each item moves to a new position in the list, shown by the arrows:

We can draw the same arrows on a single row of items, showing where each item goes in the permutation:

The structure of the permutation becomes clear if we rearrange the items so that none of the arrows are crossing. Every permutation consists of one or more cycles: in this case there is a 1-cycle sending 6 to itself, a 2-cycle swapping 3 and 5, and a 7-cycle sending 4 to 9 to 2 to 1 to 8 to 7 to 0 to 4.

It takes \$n-1\$ swaps to implement a cycle of length \$n\$. So if the sorted permutation of a list can be decomposed into cycles with lengths \$a, b, c, \ldots, z\$ then the minimum number of swaps needed to sort the list is \$(a - 1) + (b - 1) + (c - 1) + \cdots + (z - 1)\$.

Here's an implementation that finds the cycles first and then uses their lengths to count the swaps. (It's not actually necessary to find the cycles, you could just count the swaps as you go along, but I think this is clearer to begin with.)

def cycle_decomposition(permutation):
    """Generate cycles in the cyclic decomposition of a permutation.

        >>> list(cycle_decomposition([7, 2, 9, 5, 0, 3, 6, 8, 1, 4]))
        [[0, 7, 8, 1, 2, 9, 4], [3, 5], [6]]

    """
    unvisited = set(permutation)
    while unvisited:
        j = i = unvisited.pop()
        cycle = [i]
        while True:
            j = permutation[j]
            if j == i:
                break
            cycle.append(j)
            unvisited.remove(j)
        yield cycle

def minimum_swaps(seq):
    """Return minimum swaps needed to sort the sequence.

        >>> minimum_swaps([])
        0
        >>> minimum_swaps([2, 1])
        1
        >>> minimum_swaps([4, 8, 1, 5, 9, 3, 6, 0, 7, 2])
        7

    """
    permutation = sorted(range(len(seq)), key=seq.__getitem__)
    return sum(len(cycle) - 1 for cycle in cycle_decomposition(permutation))


Simple Job Scheduler Class I've created a simple daily task scheduler but I'd like to change this to a general task scheduler accepting more advanced schedules.

I'd like to have some critiques from other sets of eyes on my current implementation before I continue.

public class ScheduleJob
{
    #region PRIVATE VARIABLES
    private readonly Timer  _timer;
    private Action          _job;
    private int             _setHour;
    private int             _setMin;
    #endregion

    #region CONSTRUCTOR
    public ScheduleJob()
    {
        _timer      = new Timer();
        _setHour    = 0;
        _setMin     = 0;
    }
    #endregion

    #region PUBLIC METHODS
    public void SetTimeofDay(int in_hour, int in_min)
    {
        _setHour = in_hour;
        _setMin = in_min;
    }
    public void SetJob(Action in_task)
    {
        _job = in_task;
    }
    public void Start()
    {
        try
        {
            int PeriodMin = GetHour() * 60 + GetMin(); //minute
            _timer.Interval = PeriodMin * 60 * 1000; //millisecond

            _timer.Elapsed += _timer_Elapsed;
            _timer.Start();
            IsRunning = true;
        }
        catch (Exception)
        {
            IsRunning = false;
        }
    }
    public void Stop()
    {
        if (_timer != null)
            _timer.Stop();

        IsRunning = false;
    }
    #endregion

    #region PRIVATE METHODS
    private void _timer_Elapsed(object sender, ElapsedEventArgs e)
    {
        _timer.Enabled = false;

        _job(); //perform the job

        int PeriodMin = 24 * 60;
        _timer.Interval = PeriodMin * 60 * 1000;
        _timer.Enabled = true;
    }
    private int GetHour()
    {
        int _nowHour = DateTime.Now.Hour;

        if (_nowHour > _setHour)
        {
            return (24 - (_nowHour - _setHour));
        }
        else
        {
            return (_setHour - _nowHour);
        }
    }
    private int GetMin()
    {
        int _nowMin = DateTime.Now.Minute;

        if (GetHour() == 0)
        {
            if ((_setMin - _nowMin) <= 0)
            {
                return (24 * 60) + Math.Abs(_setMin - _nowMin);
            }
        }

        return (_setMin - _nowMin);
    }
    #endregion

    #region PROPERTIES
    public bool IsRunning { get; set; }
    #endregion
}

Just a small comment from me.  I'm not so sure about the need for the method SetJob().  It seems like there is a requirement for this method to be called before the Start() and once called you wouldn't call it again?

Hence perhaps I might look at moving this to the constructor that would hopefully imply this concept?  Also that way you can't change the job.  This is of course made with the assumption you aren't looking at changing jobs once a new ScheduledJob has been setup.

Also probably what Stylecop might pick up anyway as suggested by Leonid;

I personally wouldn't recommend using an _ for a variable within a method i.e. _nowMin
Same applies for the private method _timer_elapsed()


Client-side validator for contact information form (name, phone, e-mail) My recent post had some things to fix. I think this should be well structured and written for you.

How does this code look to you? If something needs to be changed please tell me.

// Gets all elements by ID's
function elem(id) {
    return document.getElementById(id);
};
//

//Check box id"RadioGroup1_0" will be checked on start up of page (Phone)
window.onload = function () {
    document.querySelector("#RadioGroup1_0").click();
    //

//Gets my Form name (form) and on submission (submit) checks the rules below...

/* first name must be greater than 0
last name must be greater than 0
email must be greater than zero and follow the email reg ex
phone must be greater than 7 but shorter than 11
*/
    var form = document.getElementById('form');
    form.onsubmit = function (e) {
        var rules = [
        ['first-name', elem('first-name').value.length > 0],
        ['last-name', elem('last-name').value.length > 0],
        ['email', elem('email').value.length > 0 && /^([a-zA-Z0-9_\.\-])+\@(([a-zA-Z0-9\-])+\.)+([a-z0-9]{2,4})+$/.test(elem('email').value)],
        ['phone', elem('phone').value.length > 7 && elem('phone').value.length < 11 ]

    ];

   //This is the (reqMsg) error, it follows the ID's listed above above and if there is an error it must be "inline" and if there is no error it will be "none",
        var valid = true;
        var firstFocus = null;
        for (var i = 0; i < rules.length; i++) {
            var parent = elem(rules[i][0]).parentNode;
            if (!rules[i][1]) {
                valid = false;
                parent.children[2].style.display = "inline";
                if (firstFocus == null) firstFocus = parent.children[1];
            } else {
                parent.children[2].style.display = "none";
            }

        }
          if (!valid) {
            firstFocus.focus();
            return false;
        }

        return true;
    };
};

// Using only alphabet keys NO Numeric for certain field boxes...
// First Name and Last Name
function onlyAlphabets(e) {
    try {
        if (window.event) {
            var charCode = window.event.keyCode;
        } else if (e) {
            var charCode = e.which;
        } else {
            return true;
        }
        if ((charCode == 64) ||
        (charCode == 32) ||
        (charCode >= 65 && charCode <=90) ||
        (charCode >= 97 && charCode <=122))
        return true;
        else return false;
    } catch (err) {
        alert(err.Description);
    }
}

/* isNumeric
*/

// This is for the phone field, only allowing NUMERIC numbers so no alphabets. It's referencing my above JS minimum 8 and maximum 10.

var specialKeys = new Array();
function IsNumeric(e) {
    var keyCode = e.which ? e.which : e.keyCode
    var ret = ((keyCode >= 48 && keyCode <= 57) || (keyCode === 8));
    return ret;
}

Validation problems
Beware of overzealous validation. It's easy to create bugs by rejecting valid input. For example:

Some people have only one name, so the one of the name fields must be allowed to be empty.
Names often contain non-ASCII characters. Validation must not tell the user their name is invalid!
Email addresses can contain more characters than you think. For example, "Hello, world!"@localhost.localdomain is a valid address. The validity rules are complex, so if you try to implement them with a regex, you will get it wrong. Fortunately you don't need to reject all invalid addresses, only the most common mistakes. So /@/ is enough validation for practical purposes.
Phone numbers can be long, because of country codes and extensions. They can also contain characters like +-()x.

You can avoid these problems by not doing much validation. Don't do unnecessary work that creates bugs!
I wouldn't bother with the event filters at all, especially for the name, where any character should be accepted.
Style
onlyAlphabets should be called onlyLetters (it allows letters, not whole alphabets!), or onlyNameChars, since it also allows some other characters.
e.which ? e.which : e.keyCode can be simplified to e.which || e.keyCode.
This conditional...
if ((charCode == 64) ||
    (charCode == 32) ||
    (charCode >= 65 && charCode <=90) ||
    (charCode >= 97 && charCode <=122))
    return true;
    else return false;

...can be simplified to:
return charCode == 64 ||
       charCode == 32 ||
       charCode >= 65 && charCode <= 90 ||
       charCode >= 97 && charCode <= 122;

...but character codes are hard to read. It might be better to use the string form instead, via operations like String.fromCharCode and charCodeAt.
There are too many blank lines. It's OK to leave one blank line to separate sections (like a paragraph break), but (like anything that consumes lines) they reduce the amount of code that fits on one screen, they should be used only when they improve clarity enough to be worth the extra scrolling.


Hashing with linear probing I am seeking a review of a previous project I completed to help understand hash tables.  The following is my implementation of a hash-table using linear probing.

My solution assumes a number of variables have been initialised:

Private int[] H;  Representing the hash table with each value initialised to -1
private int hSize; Storing the size of the hash table

Hash Function:

public int hash(int key){
    return key % hSize;
}

Add value to hash table:

private void addToHash(int value){

    int i;

    for (i = hash(value); H[i] != -1; i = (i + 1) % H.length)
        if (H[i] == value) return;

    H[i] = value;

}

Search for value:

public int hashSearch(int key){

    int hash = hash(key);

    for (int i = hash; H[i] != -1; i = (i + 1) % H.length)
        if (H[i] == key) return i;

    return -1;

}

I am looking for feedback regarding the design of my solution / programming style - Anything which could be improved in the future or would not be encouraged when programming in the industry.

The method names are a little bit misleading. They're already in the HashTable class, aren't they? If yes, then why would you add the hash word to their names (except for the one that actually computes the hash)? hashSearch and addToHash make it sound like the code looks for/adds a hash. It doesn't. It adds/searches for the value.
What does the hashSearch return? The index is the internal implementation detail. I do not see a good reason to expose it to the client. It should probably return a boolean value (and I'd call it something like contains).
You did not present the full code, so there are a lot of questions left. What is the initial number of buckets in the table? Does it grow? What happens if it gets fully loaded (the addToHash method can stuck in an infinity loop in that case)? These questions are very important, but, unfortunately, I can't comment on those as the relevant parts of your code are missing.


Detecting cycle in LinkedList Is this a fair/clean way to detect a cycle in a LinkedList? Should work for both single and double LinkedLists'.

public bool IsCircular()
{
    if (Head != null && Head.Next != null)
    {
        var slow = Head;
        var fast = Head.Next;
        while (slow.Next != null && fast.Next != null && fast.Next.Next != null)
        {
            if (slow == fast)
            {
                return true;
            }
            slow = slow.Next;
            fast = fast.Next.Next;
        }
        return false;
    }
    else
    {
        return Head != null ? (Head == Head.Next) : false;
    }
}

It detects a cycle very nicely. However, I've got a few remarks:

Personally I would rename your heads to slowHead and fastHead. The expression slowHead == fastHead makes more sense then.

Your else block will always return false though. So you can get rid of that and replace it with return false. If you do that, the return false after the while loop can be removed as well.


MEF Execute Plugin Exports in Threads I am developing a service application that would implement a peer to peer messaging system. I am wondering if my following code is the best approach. It works, but I am naive and it would be nice to have validation from experts for performance and reliability.

There would be about 5 plugins and a large number of messages (over 100,000 in a day. Each message could be upto 1000 characters) may be generated by each plugin and communicated to each other based on some logic.

The following are the MEFContracts -

namespace MEFContracts{

//peer to peer message transmissions.  Any plugin or core can transmit messages to one another
public interface IPeerMessage
{
void ReceiveMessage(string message,string from,string to);
}

//Core application starts a thread on the following exported plugin function
public interface IExecuteThread
{
void ExecuteThread();
}
}

The following is the Core application export -

namespace MEFCore{
[Export(typeof(IPeerMessage))]
[ExportMetadata("MEFName", "Core")]

class MEFExports:IPeerMessage
{

public void ReceiveMessage(string message, string from, string to)
{
    log.Info("Message Received by Core :" + message + from + to);
}
}
}

The following is each plugin code.

namespace MEFPlugin{
[Export(typeof(IPeerMessage))]
[ExportMetadata("MEFName", "PlugIn")]
[Export(typeof(IExecuteThread))]

public class MEFPluginExports : IPeerMessage, IExecuteThread
{
[ImportMany]
public System.Lazy<IPeerMessage,
IDictionary<string, object>>[] _peerMessageComponents
{ get; set; }

public void ReceiveMessage(string message, string from, string to)
{
    log.Info("Message Received by Plugin:" + message + " " + from + " " + to);
}

public void ExecuteThread()
{
//The core application starts this thread which generates messages.  It then transmits these messages to the various parts based on the MEFName attribute
while(true) //I will implement later a way for the core to stop the thread when the service is stopped
{
   //generate message
   //Determine the part to communicate to, say _core

    _core.ReceiveMessage("Hello Core", "PlugIn", "Core");

}
}

And the following is the main core service

[ImportMany]
public System.Lazy<IPeerMessage,
IDictionary<string, object>>[] _peerMessageComponents
{ get; set; }
[ImportMany(typeof(IExecuteThread))]
public IEnumerable<IExecuteThread> _pluginThreads { get; set; }

public void Process()
{
    log.Info("MEFCoreService started");
    AssembleMEFComponents();

     foreach(IExecuteThread _pluginThread in _pluginThreads)
    {
        Thread _thread = new Thread(_pluginThread.ExecuteThread));
        _thread.Start();
        _thread.Join();

    }

}

public void AssembleMEFComponents()
{
    try
    {
        //Creating an instance of aggregate catalog. It aggregates other catalogs
        var aggregateCatalog = new AggregateCatalog();

        //Build the directory path where the parts will be available
       //using the directory catalog
        var directoryCatalog = new DirectoryCatalog(directoryPath, "*.dll");

        //Load parts from the current assembly
        var asmCatalog = new AssemblyCatalog(Assembly.GetExecutingAssembly());

        //Add to the aggregate catalog
        aggregateCatalog.Catalogs.Add(directoryCatalog);
        aggregateCatalog.Catalogs.Add(asmCatalog);

        //Crete the composition container
        var container = new CompositionContainer(aggregateCatalog);

        // Composable parts are created here
        container.ComposeParts(this);
    }
    catch (Exception ex)
    {
        throw ex;
    }
}
}

Only a quick shot at AssembleMEFComponents() method

declare your variables as near to their usage as possible.
using try..catch like you do is not only superfluous but also breaks the stacktrace.
comments like //Add to the aggregate catalog are superfluous too. Comments should describe why something is done in the way it is done. What is done should be described by the code itself by using meaningful and descriptive name for methods, variables and classes.

That being said your method should look like so

public void AssembleMEFComponents()
{

    var directoryCatalog = new DirectoryCatalog(directoryPath, "*.dll");

    var assemblyCatalog = new AssemblyCatalog(Assembly.GetExecutingAssembly());

    var aggregateCatalog = new AggregateCatalog();
    aggregateCatalog.Catalogs.Add(directoryCatalog);
    aggregateCatalog.Catalogs.Add(assemblyCatalog );

    var container = new CompositionContainer(aggregateCatalog);

    container.ComposeParts(this);
}


Choosing a magic number between 0 and 9 I wrote a little script that asks the user to choose a number between the 0 and 9. Wile he/she is entering a choice, the program tells him/her if the number is too high and too low.

Nothing fancy, just a couple of while and ifs. It's very ugly and has imperative code with mixed responsibilities. I was wondering if there are some Ruby idioms that would make this code more readable and compact. I'm looking for some functions like in functional languages, or maybe there are special Ruby functions I'm not aware off. I don't want to define functions and classes for this simple program.

keepGoing = true
numberToGuess = rand(10)
firstRun = true
response = ''
while keepGoing
    if firstRun
        puts 'Find the magic number between O and 9'
        firstRun = false
    end
    proposedN = gets.chomp.to_i
    if proposedN == numberToGuess
        firstRun = true
        puts "You found the correct number : #{numberToGuess}"
        puts 'Another try ? (Y/N)'
        numberToGuess = rand(10)
        response = gets.chomp
        while response != 'N' and response != 'Y'
            puts 'Please fill in either Y or N'
            response = gets.chomp
        end
        response == 'N' ? keepGoing = false : keepGoing = true
        response = ''
    else
        if proposedN > numberToGuess
            puts 'Too high'
        else
            puts 'Too low'
        end
    end
end

Well, you already identified the main issue with your code, and I concur: the imperative style. Also, note that this code should have two loops and you are doing it all in one, that's why you have to use flags all over the place. Since you asked for a functional solution, no while nor loop,  use recursion:

def guess_number(number_to_guess)
  guess = gets.to_i
  if guess == number_to_guess
    puts("You found the correct number: #{number_to_guess}")
  else
    puts(guess > number_to_guess ? "Too high" : "Too low")
    guess_number(number_to_guess)
  end
end

def game
  puts('Find the magic number between O and 9')
  guess_number(rand(10))
  print('Another try? (Y/N): ')
  if gets.strip.downcase == "y"
    game
  end
end

game

If you want to validate the input, create a method def get(validation_regexp) and use it: get(/^\d+$/) and get(/^[yYnN]$/).


Abstracting device IO / glue logic in a CPU emulator In my Motorola 6809 CPU emulator (written in C++14) I'm working on an abstraction to represent the wiring between emulated devices, such as the ports on a 6522 VIA, or the interrupt lines connecting a peripheral to the CPU.
The abstraction uses functional composition.  Each OutputPin encapsulates a developer-supplied function that returns the current state of that output.  InputPin objects belonging to a different device have a copy of the OutputPin to which they are attached.  Each InputPin may only be attached to a single OutputPin, but an OutputPin may be attached to multiple InputPins.
This abstraction is working, and allows for expressive and powerful constructions like this, which does a "wired-and" of three IRQ lines (where one of them is active-high) and attaches it to the CPU's IRQ input and each time the state of that line is tested the composed functions generate the correct result based on the current state of the three inputs:
cpu.IRQ << (!fdc.DIRQ & via.IRQ & acia.IRQ);

and this, which generates an output signal that is the parity of 8 other outputs.
OutputPin parity = out[0] ^ out[1] ^ out[2] ^ out[3] ^ out[4] ^ out[5] ^ out[6] ^ out[7];

My concern, though, is performance.  I had hoped that compilers might be able to elide many of the function calls and collapse them into simpler expressions but initial tests with clang 12.0 on macOS don't appear to show any sign of this.
Is there anything I could be doing to improve the run-time efficiency, or am I hoping for too much from the compiler's optimiser?
Here's the complete implementation as a header with inline functions:
static inline bool default_true() {
        return true;
}

class OutputPin {

public:
        using Function = std::function<bool()>;

protected:
        Function        f = default_true;

public:
                        OutputPin() { }

                        OutputPin(const Function& f) : f(f) { }

        void            bind(const Function& _f) {
                                f = _f;
                        }

                        operator bool() const {
                                return f();
                        }

        OutputPin       operator !() const {
                                return OutputPin([&]() {
                                        return !f();
                                });
                        }
};

class InputPin {

protected:
        OutputPin       input;

public:
        void            attach(const OutputPin& _input) {
                                input = _input;
                        }

        operator        bool() const {
                                return input;
                        }
};

inline void operator<<(InputPin& in, const OutputPin& out)
{
        in.attach(out);
}

inline OutputPin operator&(const OutputPin& a, const OutputPin& b)
{
        return OutputPin([=]() {
                return (bool)a && (bool)b;
        });
}

inline OutputPin operator|(const OutputPin& a, const OutputPin& b)
{
        return OutputPin([=]() {
                return (bool)a || (bool)b;
        });
}

inline OutputPin operator^(const OutputPin& a, const OutputPin& b)
{
        return OutputPin([=]() {
                return (bool)a ^ (bool)b;
        });
}

and here's a trivial test case for the above header (but which generates 60kB of assembler output!):
#include "device.h"

int main()
{
        OutputPin   a, b, c;
        InputPin    i;

        i << (!a & b & c);

        return i;
}

Overhead of std::function
The main problem is that you are using std::function, and this comes with some overhead. In particular, it will allocate storage (using new internally) since your lambdas are capturing variables. Since memory allocations can have side effects (they might even throw exceptions if memory could not be allocated), the compiler cannot optimize them away.
Consider just storing the state of a pin as a  bool
The state of an output pin is just true or false. Instead of storing a function that calculates the state, consider just storing the current state in a bool. It seems unlikely to me that always ensuring the state of the pin is set correctly is less efficient than having a function that calculates it whenever you need the value. An InputPin should then not store a copy of an OutputPin, but either a reference to an OutputPin, or if you want to still be able to have an expression assigned to an InputPin, have it store the function that calculates the input value.
Once you do this, everything simplifies enormously, and the compiler will have no trouble optimizing this code.
Here is an example:
#include <functional>

using OutputPin = bool;

template<typename Function>
class InputPin {
    Function f;

public:
    InputPin(const Function &f): f(f) {}

    operator bool() const {
        return f();
    }
};

int main()
{
        OutputPin   a(true), b(true), c(true);
        InputPin    i([&]{return !a & b & c;});

        return i;
}


Using Concurrent Dictionary and Lazy to cache expensive query results and only run query once in a threadsafe manner Ok, so I'm querying a webservice. This webservice is slow with multiple o's. I want to cache the results of the query, because I only want to query on a given set of parameters once during my transaction (In addition to the query being slow, it's also rate limited, so I don't want to perform unnecessary queries). Finally, to speed things up, I want to be able to run queries in parallel, as latency is my biggest problem here.

With all of that, I think I need a thread safe way to cache data and only run the queries I actually need to run. I think I've managed to cobble that together with Concurrent Dictionary and Lazy

private static ConcurrentDictionary<KeyObject, Lazy<IEnumerable<ValueObject>>> DataCache =
new ConcurrentDictionary<KeyObject, Lazy<IEnumerable<ValueObject>>>();

    public static IEnumerable<ValueObject> GetData(KeyObject key, QueryObject query)
    {
        var value = new Lazy<IEnumerable<ValueObject>>(() =>
        {
            return query.Run();
        });
        DataCache.TryAdd(key, value);
        return DataCache[key].Value;
    }
}

If I'm correct in understanding how all the moving parts work, it should go something like this.

Generate dictionary key and Lazy intializer for value
Try and add lazy initializer to dictionary with key. If another thread has already added key, fail and continue.
Try and get value of lazy intializer on record in the dictionary. If another thread is already getting value, block until other thread has retrieved value.

Does this sound correct?

What you have will work, but there are a few incremental improvements.  Rather than using TryAdd you can use GetOrAdd which returns the value for that key.  This will save you a lookup (and also remove a race condition where the key is removed between calls by another thread).

Also note that, currently, you're caching any errors that you get, and continuing to serve them out.  If you're okay with that, or aren't worried about it, then okay.

You should do some tests to be sure, but it may be faster to use the overload of GetOrAdd that accepts a function, rather than a value.  This will allow you to construct the Lazy only when a new one is needed, rather than on every call.  Constructing a Lazy isn't that expensive (it doesn't need to actually compute the value after all) but this is likely still a win in most situations.


Simple Python Code that zips file My Code
import shutil
import os
from pathlib import Path
from datetime import datetime

DOWNLOAD_DIR = Path("/Users/my_name/Desktop/Zip Test/g_2020-12-10_3/")
ZIPPED_DIR = Path("/Users/my_name/Desktop/Zip Test/g_2020-12-10_3.zip")
ZIPPED_DIR.mkdir(exist_ok=True)

def make_archive(source, destination):
    delivery_file_name = destination.split("/")[-1]
    delivery_name = delivery_file_name.split(".")[0]
    format = delivery_file_name.split(".")[1]
    archive_from = os.path.dirname(source)
    archive_to = os.path.basename(source.strip(os.sep))
    shutil.make_archive(delivery_name, format, archive_from, archive_to)
    shutil.move(
        "%s.%s" % (delivery_name, format),
        "/Users/my_name/Desktop/Zip Test/zipped_content",
    )

make_archive(str(DOWNLOAD_DIR), str(ZIPPED_DIR))

Description
I am very new to python so i had a very hard time getting shutil.make_archive to do what i want. This code is finally working and i feel it is a little hard to read so i was wondering if someone can help me refactor this to simplify it. I don't want to loose the functionality of what i am doing. In my code i basically go to my /Users/my_name/Desktop/Zip Test and inside it is a folder called g_2020-12-10_3 which contains the contents i want to zip up so i do that.
When i unzip the contents it unzips folder named g_2020-12-10_3 and inside it are the contents.
I want to keep this functionality but at the same time simplify the code below and any help would be appreciated here.

You're using pathlib, which is great! Unfortunately, you immediately discard the functionality it gives you, and use old-style os APIs.
A good place to start is here - a mapping between the os[.path] APIs and their Path equivalents. Doing that gets us here:
def make_archive2(source, destination):
    shutil.make_archive(destination.stem, destination.suffix, source.parent, source.name)
    shutil.move(destination, "/Users/my_name/Desktop/Zip Test/zipped_content")

We now don't need to do any manual manipulations using os or os.path. Once we're here, we can actually realize that the first parameter to make_archive is a full path to the file, completely removing the requirement to call shutil.move
From there, we can improve some of the other things you do. For example:

Your source and destination are the same thing... but you don't actually use the destination in a meaningful way
You extract the format from the destination, which feels a bit odd
You hardcode the actual final destination
You use both the root_dir and base_dir parameters, when that really isn't necessary
You don't return where the file ended up, which feels nicer than the caller having to set up their Path themselves

Altogether, I ended up with something like this:
def make_archive3(to_archive, move_to, archive_format="zip"):
    move_to.mkdir(exist_ok=True)
    return shutil.make_archive(move_to / to_archive.name, archive_format, to_archive)


Password Bruteforcer Just an idea I had for a Java exercise. Cracking a password like "testing" takes at least 15 minutes, so obviously it's not as efficient as it can be. What are your thoughts? I also never worked with multithreading, so I can imagine I could be doing things better there.

public class AlgoTester implements Runnable {
    private static final char[] possibleChars = { 'a', 'b', 'c', 'd', 'e', 'f', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o',
            'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z', '0', '1', '2', '3', '4', '5', '6', '7', '8', '9' };
    private static int[] arrayIndices;
    private static String testPassword = "testing";
    private static final int passwordLength = testPassword.length();
    private static boolean foundPassword = false;
    private final static int threadCount = 5;
    private static String passwordFound = "";

    public static void main(String[] args) {
        arrayIndices = new int[passwordLength];
        for (int i = 0; i < arrayIndices.length; i++) {
            arrayIndices[i] = 0;
        }
        for (int i = 0; i < threadCount; i++) {
            new Thread(new AlgoTester()).start();
        }

    }

    private synchronized static boolean advanceIndicies() {
        synchronized (arrayIndices) {
            if (foundPassword)
                return false;
            int maxValue = possibleChars.length - 1;
            for (int i = 0; i < passwordLength; i++) {
                int inverse = arrayIndices.length - i - 1;
                if (arrayIndices[inverse] == maxValue) {
                    if (inverse - 1 == -1)
                        return false;
                    arrayIndices[inverse] = 0;
                    continue;
                } else {
                    arrayIndices[inverse] = arrayIndices[inverse] + 1;
                    return true;
                }
            }
            return false;
        }
    }

    private synchronized static String genPassword() {
        synchronized (arrayIndices) {
        StringBuilder builder = new StringBuilder();
        for (int i : arrayIndices) {
            builder.append(possibleChars[i]);
        }
        String returning = builder.toString();
        System.out.println(Thread.currentThread().getName() + ": Checking: " + returning);
        return returning;
    }
    }

    private static boolean checkPassword() {
        // TODO Auto-generated method stub
        return (foundPassword = genPassword().equals(testPassword));
    }

    @Override
    public void run() {
        while (!foundPassword) {
            if (checkPassword()) {
                passwordFound = genPassword();
                System.out.println("----------------------");
                System.out.println("Found Password: " + passwordFound);
                System.out.println("----------------------");
                foundPassword = true;
            }
            if (!advanceIndicies()) {
                return;
            }
        }

    }

}

It's hard to decide where to begin. OK: no comments whatsoever. You know how it works now, but it will not be obvious next month if you do anything but sit idly in a dark room being fed through a tube thinking about this code.

Indicies is not a word. Don't write code on a napkin, everything can spell check English nowadays.

  I also never worked with multithreading, so I can imagine I could be doing things better there.

It is best to learn how (and when) to do it and then try and come here. Don't just write something that compiles and throw at our face.

It is easier on the brain if you split this on a class that is your "driver", "main", "starter" which has a main() and does the instantiation and invocations and another one that is instantiated and called. I am not talking about correctness, obviously both approaches work, but the one I described above is clearer and wastes less cognitive power.

You forgot you were writing Java and accidentally wrote a C return statement.

return (foundPassword = genPassword().equals(testPassword));

You could afford another line, your IDE wouldn't run out of memory:

foundPassword = genPassword().equals(testPassword);
return foundPassword;

Responsibilities are a salad. See that advanceIndices checks if the password has been found. It was supposed to advance the indices but it is doing way more than that.

Performance-wise, the cost of synchronization is too high. A correct approach would be to partition the problem in n subproblems - where n is the number of threads you have - of almost the same size and solve each one in its own thread, with less synchronization.

Hypothesize that you have an array with 40 valid characters and 5 threads. Make thread A check all possible passwords that start with one of the first eight characters, thread B check all possible passwords that start with one of the next eight characters and so on until thread E.

Then throw in a volatile boolean flag that indicates termination which all threads refer to every now and then to know if they can stop because a thread found a solution.

It needs to be volatile so that any thread that reads it will see the most recently written value. You should know about this before attempting multithread programming in Java.

Summing it up

Learn how to do it before bashing your head against the wall.
Parallelization is good for this problem but you did it wrong, see the item above.


Is this code a good solution for this task? interface IJuiceable
{
    void MakeJuice();
}
interface ISliceable
{
    void MakeSlices();
}

class Fruit:IJuiceable,ISliceable
{
    public string Name { get; set; }
    public void MakeJuice()=>Console.WriteLine($"Juice from {Name}");

    public void MakeSlices()=>Console.WriteLine($"Slices of {Name}");
}

class Fresh
{
    List<IJuiceable> juices = new List<IJuiceable>();
    List<ISliceable> slices = new List<ISliceable>();

    public void AddToJuices(IJuiceable fruit)=>juices.Add(fruit);
    public void AddToSlices(ISliceable fruit)=> slices.Add(fruit);

    public void MakeFreshes()
    {
        juices.ForEach(x => x.MakeJuice());
        slices.ForEach(x => x.MakeSlices());
    }
}

You have not implemented exception handling (the last requirement). Eg. if calling code passes null to AddToJuices or AddToSlices, it won't crash straight away, but as soon as you call MakeFreshes, it will result in an unhandled NullReferenceException once it gets to this null "fruit".

Also note that we don't know what calling code may pass in as an argument - could be some object that implements one of the interfaces, but actually explodes once our code calls its MakeJuice or MakeSlices... The requirement states that it's the responsibility of this code to handle exceptions, so if this was my homework, I'd prefer to be on the safe side here.

Less importantly, you implemented Name as a property of Fruit rather than a field (which is what the description asks for), although public
fields actually are considered a bit of a code smell, and a backing field is automatically created for properties, so I would leave it.
Aiming at good code style you could make it immutable though (as a readonly field, or a property with a private setter), and set it in the constructor - it stands to reason that name of a given fruit isn't subject to change. juices and slices could be readonly too, but that's nitpicking already.

Apart from the lack of exception handling, it seems okay to me. I like that you used some modern C# features to cut down on verbosity.


Leetcode online Find the judge, graph with highest degree Here is the problem.

  In a town, there are N people labelled from 1 to N.  There is a rumor
  that one of these people is secretly the town judge.

  If the town judge exists, then:

  The town judge trusts nobody.

  Everybody (except for the town judge) trusts the town judge.

  There is exactly one person that satisfies properties 1 and 2.

  You are given trust, an array of pairs trust[i] = [a, b] representing
  that the person labelled a trusts the person labelled b.

  If the town judge exists and can be identified, return the label of
  the town judge.  Otherwise, return -1.

  Example 1:

  Input: N = 2, trust = [[1,2]]
  Output: 2
  Example 2:

  Input: N = 3, trust = [[1,3],[2,3]]
  Output: 3
  Example 3:

  Input: N = 3, trust = [[1,3],[2,3],[3,1]]
  Output: -1
  Example 4:

  Input: N = 3, trust = [[1,2],[2,3]]
  Output: -1
  Example 5:

  Input: N = 4, trust = [[1,3],[1,4],[2,3],[2,4],[4,3]]
  Output: 3

  Note:

  1 <= N <= 1000
  trust.length <= 10000
  trust[i] are all different
  trust[i][0] != trust[i][1]
  1 <= trust[i][0], trust[i][1] <= N

And here is my solution.

(Logic :- Check the degree of every node, +1 for incoming and -1 for outgoing. If any node is having degree as N-1 then that is the node.)

public int findJudge(int N, int[][] trust) {
        // Create graph of N and then check degree, should be N-1
        final int NOT_FOUND = -1;
        final int trustArrayLength = trust.length;

        // Degree array should have value starting from 1 to N+1
        final int[] degreeArray = new int[N + 1];

        for (int i = 0; i < trustArrayLength; i++) {
            int[] itemInTrustArray = trust[i];

            // Since its outbound connection, decrease the degree by 1.
            degreeArray[itemInTrustArray[0]]--;

            // Since its inbound connection, increase the degree by 1.
            degreeArray[itemInTrustArray[1]]++;
        }

        // Now iterate though the degreeArray to find the index having degree as N-1.
        for (int i = 1; i <= N; i++) {
            if (degreeArray[i] == N - 1) {
                return i;
            }
        }
        return NOT_FOUND;
    }

Pleae let me know, the area of improvement.

This is a wonderfully succinct solution. I will make one point:

final int[] degreeArray = new int[N + 1];

This creates a never-used int at degreeArray[0]. I understand that this was a choice so as to be able to use a simple access by value of the trustees:

degreeArray[itemInTrustArray[0]]--;

In the interest of creating the minimum number of objects necessary, and thus using the least memory possible, I would recommend initializing degreeArray to length N

final int[] degreeArray = new int[N];

And then left shifting your insert by value statements

degreeArray[itemInTrustArray[0]--]--;
degreeArray[itemInTrustArray[1]--]++;

and finally updating your final for loop to account for this change to the zero-based indexing inherent to arrays

// Now iterate though the degreeArray to find the index having degree as N-1.
for (int i = 0; i < N; i++) {

Since you are working with int primitives, the math operators here would add only a near-vanishing amount to overall runtime, if that is a concern.


Racket code to find missing ranges I'm doing some toy problems in a variety of languages and wanted to do the following in Racket Lisp.

Given a sorted integer array nums, where the range of elements are in the inclusive range [lower, upper], return its missing ranges.
0, 1, 3, 50, 75

Lower: 0
Upper: 99
Expected Output:
["2", "4->49", "51->74", "76->99"]

As you can see on the linked page I had to take two whacks at it but eventually came up with the following. How does it look? Is it idiomatic? Could I have done certain things far better?
(define (missing-ranges nums lower upper)
  (in-generator
   (define (yield-missing-range current num)
     (yield (if (eq? num (add1 current))
                (~a current)
                (~a current "->" (sub1 num)))))
   (let recur ([remaining-nums nums]
               [current lower])
     (match remaining-nums
       [(cons num rest-nums) (if (< current num)
                                 (begin
                                   (yield-missing-range current num)
                                   (recur rest-nums num))
                                 (recur rest-nums (add1 current)))]
       ['() (cond [(<= current upper)
                   (yield-missing-range current (add1 upper))])]))))

Your solution seems to me quite complex, with iterations, recursions, and the use of match.
Here is an alternative version with a simple linear recursion, where the result is given with lists and not with strings.
(define (missing-ranges nums lower upper)
  (cond ((> lower upper) '())
        ((null? nums) (list (list lower upper)))
        ((= lower (car nums)) (missing-ranges (cdr nums) (+ lower 1) upper))
        ((= (car nums) (+ 1 lower)) (cons (list lower) (missing-ranges nums (+ lower 1) upper)))
        (else (cons (list lower (- (car nums) 1)) (missing-ranges (cdr nums) (+ (car nums) 1) upper)))))

(missing-ranges '(0 1 3 50 75) 0 99)
'((2) (4 49) (51 74) (76 99))

(missing-ranges '(8) 0 99)
'((0 7) (9 99))

(missing-ranges '() 0 99)
'((0 99))

(missing-ranges '(3 9 88 99) 0 99)
'((0 2) (4 8) (10 87) (89 98))

(missing-ranges '(0 99) 0 99)
'((1 98))

(missing-ranges '(1 2 3) 1 3)
'()