Averaging students' scores I'm still a beginner, so I'm still learning. I'm trying to take test scores for 3 different students, enter them in an array, then display the name, test scores and the average. Trying to also have reusable code for the average. I'm not even sure I'm going about it the right way. This is what I've got so far. It works, but its probably sloppy. I'm sure there are other ways of doing it, but some constructive criticism would be nice.

static void Main(string[] args)
    {
        const int STUDENTS = 3;
        string[] names = new string[STUDENTS];
        names[0] = "Morgan";
        names[1] = "Bowie";
        names[2] = "Ananya";

        Console.WriteLine("Enter test scores for " + names[0]);
        const int MORGAN_TEST = 3;
        int[] morganScores = new int[MORGAN_TEST];

        morganScores[0] = int.Parse(Console.ReadLine());
        morganScores[1] = int.Parse(Console.ReadLine());
        morganScores[2] = int.Parse(Console.ReadLine());
        int sumMorgan = morganScores.Sum();

        Console.WriteLine("Enter test scores for " + names[1]);
        const int BOWIE_TEST = 5;
        int[] bowieScores = new int[BOWIE_TEST];

        bowieScores[0] = int.Parse(Console.ReadLine());
        bowieScores[1] = int.Parse(Console.ReadLine());
        bowieScores[2] = int.Parse(Console.ReadLine());
        bowieScores[3] = int.Parse(Console.ReadLine());
        bowieScores[4] = int.Parse(Console.ReadLine());
        int sumBowie = bowieScores.Sum();

        Console.WriteLine("Enter test scores for " + names[2]);
        const int ANANYA_TEST = 4;
        int[] ananyaScores = new int[ANANYA_TEST];

        ananyaScores[0] = int.Parse(Console.ReadLine());
        ananyaScores[1] = int.Parse(Console.ReadLine());
        ananyaScores[2] = int.Parse(Console.ReadLine());
        ananyaScores[3] = int.Parse(Console.ReadLine());
        int sumAnanya = ananyaScores.Sum();

        double[] average = { MORGAN_TEST, BOWIE_TEST, ANANYA_TEST };

        Console.WriteLine(names[0] + " " + morganScores[0] + " " + morganScores[1] + " " + morganScores[2] + " Average score: {0:N2}", sumMorgan / average[0]);
        Console.WriteLine(names[1] + " " + bowieScores[0] + " " + bowieScores[1] + " " + bowieScores[2] + " " + bowieScores[3] + " " + bowieScores[4] + " Average score: {0:N2}", sumBowie / average[1]);
        Console.WriteLine(names[2] + " " + ananyaScores[0] + " " + ananyaScores[1] + " " + ananyaScores[2] + " " + ananyaScores[3] + " " + " Average score: {0:N2}", sumAnanya / average[2]);
        Console.Read();
    }

You've got students and their scores here. I assume that student without scores is meaningless, as well as scores without students. It would be nice to combine that data into single object:
public class StudentScores {

    // Auto-property guarantees that student name defined once in constructor
    // and will not accidentally change.
    public string Name { get; }

    // Scores count can be different for different students.
    // Private field guarantees that scores will not be messed up by other classes.
    // Otherwise some student could access scores and change them,
    // which would lead to errors.
    private readonly List<int> scores = new List<int>();

    public StudentScores(string name) {
        Name = name;
    }

    // We still need a method to add scores to our encapsulated list.
    // We could implement Clear() or RemoveAt() public methods later.
    public void AddScore(int score) {
        scores.Add(score);
    }

    public double Average() {
        if (scores.Count > 0)
            return scores.Average();
        else
            return 0;

        // Or simple: return scores.Count > 0 ? scores.Average() : 0;
    }

    // Nicer way to print data: student name and all scores separated by empty space.
    public override string ToString() {
        return $"{Name} {string.Join(" ", scores)}";
    }
}

Without comments it's only 20 lines long:
public class StudentScores
{
    public string Student { get; }

    private readonly List<int> scores = new List<int>();

    public StudentScores(string student)
        => Student = student;

    public void AddScore(int score)
        => scores.Add(score);

    public double Average()
        => scores.Count > 0 ? scores.Average() : 0;

    public override string ToString()
        => $"{Student} {string.Join(" ", scores)}";
}

Now your syntax in Main is much nicer:
static void Main(string[] args) {

    string[] names = {"Morgan", "Bowie", "Ananya"};

    var morgan = new StudentScores(names[0]);

    // Determine how much scores has student.
    Console.WriteLine("Enter number of test scores for " + names[0]);
    int count = int.Parse(Console.ReadLine());

    for (int i = 0; i < count; i++) {
       morgan.AddScore(int.Parse(Console.ReadLine()));
    }

    var bowie = new StudentScores(names[1]);
    // ...

    var ananya = new StudentScores(names[2]);
    // ...

    Console.WriteLine(morgan.ToString() + " Average score: {0:N2}", morgan.Average());
    Console.WriteLine(bowie.ToString() + " Average score: {0:N2}", bowie.Average());
    Console.WriteLine(ananya.ToString() + " Average score: {0:N2}", ananya.Average());
}

You could improve your Main() further creating single loop for all students:
static void Main(string[] args) {

    string[] names = {"Morgan", "Bowie", "Ananya"};
    List<StudentScores> scores = new List<StudentScores>();

    foreach(var name in names)
    {
        var student = new StudentScores(name);
        Console.WriteLine($"Enter number of test scores for {name}");
        int count = int.Parse(Console.ReadLine());
        for (int i = 0; i < count; i++)
           student.AddScore(int.Parse(Console.ReadLine()));
        scores.Add(student);
    }

    foreach(var score in scores)
        Console.WriteLine($"{score.ToString()}, Average score: {score.Average():N2}");

You could remove Average() method and move calculations directly to ToString(), but I think it's nice to have methods for such things.


Getting keyboard input for real-time console application I've been building an IDE using C# and only the console. To get keyboard input in real time (as opposed to a read–eval–print loop,) I have the following method to get keyboard input:
public static ConsoleKeyInfo[] GetInput()
{
    // A list of characters
    List<ConsoleKeyInfo> input = new List<ConsoleKeyInfo>();

    // Loop while keys are available or we hit 10 keys
    for (int i = 0; Console.KeyAvailable && i < 10; i++)
    {
        // Read a key (preventing it from being printed)
        // and put it in the key list (if it's not in there yet)
        ConsoleKeyInfo info = Console.ReadKey(true);
        if (!input.Contains(info))
        {
            input.Add(info);
        }
    }

    // Use up any remaining key presses
    while (Console.KeyAvailable)
    {
        // Read a single key
        Console.ReadKey(true);
    }

    // Convert the list to an array and return
    return input.ToArray();
}

GetInput called once at the start of the main loop and the result is passed into objects that handle key presses:
ConsoleKeyInfo[] keysHit = ConEx.ConEx_Input.GetInput();
Program.Interpreter.Update(Program.Interpreter.CurMode, keysHit);
Program.WindowUI.Update(Program.Interpreter.CurMode, keysHit);
Program.WindowSideBar.Update(Program.Interpreter.CurMode, keysHit);

Inside an Update method:
for (int i = 0; i < keysHit.Length; i++)
{
    switch (keysHit[i].Key)
    {
        //Cycle through the pages
        case ConsoleKey.Home:
            //Do some stuff
            break;
        case ConsoleKey.End:
            //Do other stuff
            break;
    }
}

Is this a sub optimal solution for getting keyboard input? I'm very willing to breakout P/Invoke or a threaded solution if need be. I can't support an event handler based solution due to the tight control of sequence needed.
Edit:
Nikita B's answer is marked as Accepted because it reduced the amount of ticks taken from ~60 to ~30. Thanks!

You could join the two loops. I think this way your intent will be clearer, and it's not like a few extra comparisons are going to cost you anything.

//return "IEnumerable<T>" if all you need to do is iterate over elements
public static IEnumerable<ConsoleKeyInfo> GetInput()
{
    //use "var" if return type is obvious from context
    //use "HashSet<T>" if you need a collection of unique items
    var input = new HashSet<ConsoleKeyInfo>();
    var keyCount = 0;
    while (Console.KeyAvailable)
    {
        var key = Console.ReadKey(true);
        //use constants instead of magic numbers
        if (++keyCount > MaxInputLength) continue;
        //no need to call "Contains", non-unique items will be dropped automatically
        input.Add(key);
    }
    //no need to convert anything
    return input;
}

But I have to question your design. Why do I have to input (and memorize?) a 10-key sequence where every key means something and the order is important? Sounds like a really poor user experience. I feel like it is not the way to go in modern applications, iddqd or not.

Edit: I forgot, that HashSet does not preserve insertion order. So if order is important, you should stick to List (your original approach).


Javascript split array into n subarrays, size of chunks don't matter NOTE: This post was moved from stackoverflow as codereview.stackexchange is a better place to discuss the performance of this code problem/solution.
I want to split an array into n subarrays.
I don't care how many elements end up in each array but the elements must be spread through all the available sub arrays.
e.g: Solutions A & B are two ways of doing it but I'm looking for Solution A:
a = [1,2,3,4,5,6,7,8,9]
into_subarrays(a, 2);

Solution A => [[1,3,5,7,9],[2,4,6,8]]
Solution B => [[1,2,3,4,5],[6,7,8,9]]

into_subarrays(a, 4);

Solution A => [[1,5,9],[2,6],[3,7],[4,8]]
Solution B => [[1,2,3],[4,5],[6,7],[8,9]]

into_subarrays(a, 6);

Solution A => [[1,7],[2,8],[3,9],[4],[5],[6]]
Solution B => [[1,2],[3,4],[5,6],[7],[8],[9]]

into_subarrays(a, 12);

Solution A => [[1],[2],[3],[4],[5],[6],[7],[8],[9],[],[],[]]
Solution B => [[1],[2],[3],[4],[5],[6],[7],[8],[9],[],[],[]]

I have this solution, I just want to make sure it's as efficient as possible:
into_subarrays(myArray, chunks=2){
  var a = myArray.slice(); //Copy array so that the original is not modified
  var i = 0;
  var result = [];

  while(a.length){
   //Create array if needed
   if (typeof result[i] == 'undefined'){
     result[i] = [];
   }

   result[i].push(a.shift());
   i++;
   i = (i == chunks) ? 0 : i; //Wrap around chunk selector
  }

  return result;
}

Thanks.

First: Since you already know how many chunks you want, there's no point in checking if the chunk exists and otherwise declare it. In general terms, you should not need to check for objects you can define deterministically.
So instead of doing this check on each loop:
if (typeof result[i] == 'undefined'){
  result[i] = [];
}

create an array of N empty arrays beforehand.
const result = Array.from(Array(chunks),item=>[]);

Second: Albeit the performance difference is negligible, checking for i's value and conditionally reassigning it is less efficient than using the modulo operator on its value regardless
So instead of
results[i].push(...)
i++
i = (i == chunks) ? 0 : i;

You can do
results[i % chunks].push(...)
i++

With the above, your function could be expressed as
function usingShift(myArray, chunks=5){
  const copiedArray = myArray.slice(),
        result=Array.from(Array(chunks),item=>[]);
  let i=0;
  while(copiedArray.length){
    result[i % chunks].push(copiedArray.shift());
    i++;
  }
  return result;
}

Third: As you've been told, shifting from an array is expensive. I understand you're doing it because you want to populate the chunks in the same order of the original array. However you can achieve the same popping from the a reversed array:
If you declare
const a = myArray.slice().reverse();

You can replace the usage of shift with
result[i].push(a.pop());

The function would be something like:
function usingPop(myArray, chunks=5){
  const reversedArr = myArray.slice().reverse(),
        result=Array.from(Array(chunks),item=>[]);
  let i=0;
  while(reversedArr.length){
    result[i % chunks].push(reversedArr.pop());
    i++;
  }
  return result;
}

However... you'd still be copying the array and performing a mutation on the copy.  @Miklós Mátyás solution has the advantage of populating the result without copying nor extracting items from the source array. Now, you haven't said the source array will be always the same (9 elements from 1 to 9). It could as well have repeated/unsorted items, so his solution should take into account not the item itself but its index, which can be expressed as:
function filterByModulo(myArray, chunks=5){
  return Array.from(Array(chunks),(_,modulo)=>{
    return myArray.filter((item,index) =>  index % chunks === modulo);
  });
}

That's pretty clean, but it's filtering on the original array as many times as chunks you want, so it's performance degrades according to the source array length AND the chunk quantity.
Personally I believe this is a case in which reduce would be more appropiate and pretty concise, while avoiding the copying or mutation of the source array.
function usingReduce(myArray,chunks=5) {
   const result=Array.from(Array(chunks),i=>[]);

   return myArray.reduce( (accum,item,index)=>{
      accum[index%chunks].push(item);
      return accum;
   }, result);
}

Finally there's the classic for loop
function classicFor(sourceArr, chunks=5) {
  const lengthOfArray=sourceArr.length;
  const result=Array.from(Array(chunks),i=>[]);

  for(let index=0; index<lengthOfArray ; index++) {
     result[index % chunks ].push(sourceArr[index]);
  }
  return result;
}

I made a test case at JSPerf in which it shows that the for loop is the most efficient. (I threw in forEach and for..of implementations too).
Running with a source array of 5000 items and 5 chunks shows that using pop on the source is more efficient than using shift by a 2.89x factor. It even looks more efficient that reduce. The classic for loop is the fastest whereas filtering N times comes up last by a ratio of 9x the modulo filtering.
If you use a source of 100000 items and 15 chunks the classic for is still the most efficient (still 9x modulo filtering) but the other implementations do scale a bit better than modulo filtering.


My Python3 Library provides a Wrapper around the Popular espeak TTS My code:

import subprocess
import os
import platform

class Speaker:
    """
    Speaker class for differentiating different speech properties.
    """

    def setVoice(self, voice):
        self.voice = voice

    def setWPM(self, wpm):
        self.wpm = wpm

    def setPitch(self, pitch):
        self.pitch = pitch

    def setProperties(self, voice="en", wpm=120, pitch=80):
        self.setVoice(voice)
        self.setWPM(wpm)
        self.setPitch(pitch)

    def __init__(self, voice="en", wpm=120, pitch=80):
        self.prevproc = None
        self.setProperties(voice, wpm, pitch)
        self.executable = os.path.dirname(os.path.abspath(__file__)) +
                              "/espeak.exe" if platform.system() == 'Windows'
                              else os.path.dirname(os.path.abspath(__file__)) + "/espeak"

    def generateCommand(self, phrase):
        cmd = [
            self.executable,
            "--path=.",
            "-v", self.voice,
            "-p", self.pitch,
            "-s", self.wpm,
            phrase
        ]
        cmd = [str(x) for x in cmd]
        return cmd

    def say(self, phrase, wait4prev=False):
        cmd=self.generateCommand(phrase)
        if wait4prev:
            try: self.prevproc.wait()
            except AttributeError: pass
        else:
            try: self.prevproc.terminate()
            except AttributeError: pass
        self.prevproc = subprocess.Popen(cmd, executable=self.executable,
                            cwd=os.path.dirname(os.path.abspath(__file__)))

How can the above code be improved?

Find it on GitHub: https://github.com/sayak-brm/espeak4py

Usage:

import espeak4py
import time

print('Testing espeak4py\n')

mySpeaker = espeak4py.Speaker()

mySpeaker.say('Testing', wait4prev=True)

print('Testing wait4prev')

mySpeaker.say('Hello, World!')
time.sleep(1)
mySpeaker.say('Interrupted!')
time.sleep(3)

mySpeaker.say('Hello, World!')
time.sleep(1)
mySpeaker.say('Not Interrupted.', wait4prev=True)
time.sleep(3)

print('Testing pitch')

myHighPitchedSpeaker = espeak4py.Speaker(pitch=120)
myHighPitchedSpeaker.say('I am a demo of the say function')
time.sleep(4)

print('Testing wpm')

myFastSpeaker = espeak4py.Speaker(wpm=140)
myFastSpeaker.say('I am a demo of the say function')
time.sleep(4)

print('Testing voice')

mySpanishSpeaker = espeak4py.Speaker(voice='es')
mySpanishSpeaker.say('Hola. Como estas?')

print('Testing Completed.')

1.PEP8

Naming

In python functions/methods/variables are using underscore as a naming separator, but not a camelCase.

Quotes

You should pick one double or single quotes and use it everywhere that is a general rule, I prefer single ones so I've changed a code as I like, but you might want to use double, just follow one way to do that.

Please read code styling guide aka PEP8

2.Class definition

Methods order

I prefer to have __init__ and __new__ as first methods defined by class, this gives you better image on class and what it does and what it got. This is not a strong rule, but most of people and libraries are following it.

Properties definition

Most of the times you want to have all your properties defined in init,
again that is not a strong rule, but in this particular case, I would define them inside __init__ instead of having a separate method to do that. If fact you I don't see much of use of your setProperties method, so I would just modify object attributes directly here if I would need it. Since there are no logics inside those methods except for changing an attribute to a value passed into it.

3.Improvements

I'm not a big fan of try:catch:pass statements you can avoid them with pre-conditions: so your say method can be modified like this:

def say(self, phrase, wait4prev=False):
    cmd = self.generate_command(phrase)
    if self.prevproc:
        if wait4prev:
            self.prevproc.wait()
        else:
            self.prevproc.terminate()
    self.prevproc = subprocess.Popen(cmd, cwd=os.path.dirname(os.path.abspath(__file__)))

Note: you don't need to pass executable explicitly since it's already a part of your cmd param.

So in the end what we have is this:

import os
import platform
import subprocess

class Speaker:
    """
    Speaker class for differentiating different speech properties.
    """
    def __init__(self, voice='en', wpm=120, pitch=80):
        self.prevproc = None
        self.voice = voice
        self.wpm = wpm
        self.pitch = pitch

        executable = 'espeak.exe' if platform.system() == 'Windows' else 'espeak'
        self.executable = os.path.join(os.path.dirname(os.path.abspath(__file__)), executable)

    def generate_command(self, phrase):
        cmd = [
            self.executable,
            '--path=.',
            '-v', self.voice,
            '-p', self.pitch,
            '-s', self.wpm,
            phrase
        ]
        cmd = [str(x) for x in cmd]
        return cmd

    def say(self, phrase, wait4prev=False):
        cmd = self.generate_command(phrase)
        if self.prevproc:
            if wait4prev:
                self.prevproc.wait()
            else:
                self.prevproc.terminate()
        self.prevproc = subprocess.Popen(cmd, cwd=os.path.dirname(os.path.abspath(__file__)))


How would I name these types in a way that's accurate and not redundant? I have a component that shows the status of a customer's billing plan. There are about seven different states it can be in, with about six props each (I've simplified it down to two for this example). I'm trying to create an enum that I can get the props from, to be used like this:
<PlanCard {...PlanCardProps.TrialPlan} />

But I'm trying to give it proper typing. The names just feel wonky here though. I hate using "type" as part of the name of a type, but given that the enum is called PlanCardProps, I just didn't know what to call the typing for the object literal and for its values.
Is there a better way of naming this?
export type PlanCardPropsType = {
  header: string;
  ctaText: string;
};

type PlanCardPropsEnum = {
  [key: string]: PlanCardPropsType;
};

export const PlanCardProps: PlanCardPropsEnum = {
  TrialPlan: {
    header: 'x',
    ctaText: 'click here'
  },
  NoPlan: {
    header: 'y',
    ctaText: 'cancel'
  }
};

Given that:

PlanCardPropsEnum is only referenced once, in the type definition of PlanCardProps
PlanCardPropsEnum is not exported
The exported object is absolutely constant (right?)
The exported object is not props, but a container for different possible props

I would refactor it to:

Rename the exported object to UPPER_SNAKE_CASE, which is acceptable for absolute constants/enums. Maybe call it AVAILABLE_PLAN_CARD_PROPS, since it's not props itself, but a container around multiple props. Or you could call it ALL_PLAN_CARD_PROPS.
Remove PlanCardPropsEnum completely. TypeScript can already automatically infer the type of the object to have values of { header: string; ctaText: string; };, and you probably shouldn't type as [key: string] because you only have "around six props each" - you'd want only those particular props to be referenceable on the object, and for any other property accesses to throw a TS error.
Then, PlanCardPropsType can be renamed to PlanCardProps:

export type PlanCardProps = {
  header: string;
  ctaText: string;
};

// This could also be called AvailablePlanCardProps
// now that its name doesn't collide with another's
export const AVAILABLE_PLAN_CARD_PROPS = {
  TrialPlan: {
    header: 'x',
    ctaText: 'click here'
  },
  NoPlan: {
    header: 'y',
    ctaText: 'cancel'
  }
};

It can often be good to avoid explicit typing - better to let TS automatically infer types. This cuts down on code that needs to be read.
It's unfortunate that props is plural; if it were singular, it would be easier to come up with a good name to distinguish the collection from the individual.
Note that "enum" is a keyword with a specific meaning in TS. Probably best only to refer to actual TS enums as "enum", else you may confuse people.
If you're worried about possibly making a typo and, for example, mis-typing headrr instead of header, you could define the object first (so that all the keys get included in the type, without repetition), then assign the object to another variable whose type is a Record of those keys and the PlanCardProps:
export type PlanCardProps = {
  header: string;
  ctaText: string;
};

// Ensure that the values of the exported object match PlanCardProps:
type Keys = keyof typeof AVAILABLE_PLAN_CARD_PROPS_INITIAL;
export const AVAILABLE_PLAN_CARD_PROPS: Record<Keys, PlanCardProps> = AVAILABLE_PLAN_CARD_PROPS_INITIAL;

This will throw a TS error if you make a typo.


Bash script to calculate when I wake up This script tries to determine the time when you woke up.

Of course, it only applies if you turn your computer on not too long ago after getting out of bed.

If not, it might still be useful as an indicator of when you turned your computer on.

It works by getting information from journalctrl, sleep-resume dates and boot dates.

It then loops through them and tries to find when the difference is above a certain gap (5 hours by default, it can be configured with the -g flag).

Then it prints the time and "timeago" information of when you probably woke up.

#!/bin/bash

gap=18000

while [ ! $# -eq 0 ]
do
    case "$1" in
        --help | -h)
            printf "Version: 1.2.1\nFlags:\n\t-g or --gap: Specifies the chunks of time between two dates to determine when you probably woke up (default is 5)\n"
            exit
            ;;
        --gap | -g)
            gap=$(($2 * 3600))
            ;;
    esac
    shift
done

current_date=$(date +%s)
readarray -t sleep_dates < <(journalctl -o short-unix -t systemd-sleep | grep resumed | tail -50 | awk -F. '{print $1}')
readarray -t boot_dates < <(journalctl --list-boots | tail -50 | awk '{ d2ts="date -d \""$3" "$4" " $5"\" +%s"; d2ts | getline $(NF+1); close(d2ts)} 1' | awk 'NF>1{print $NF}')
dates=( "${sleep_dates[@]}" "${boot_dates[@]}" )
readarray -t sorted_dates < <(printf '%s\n' "${dates[@]}" | sort)

for (( i=${#sorted_dates[@]}-1 ; i>=0; i-- )); do
    diff=$((sorted_dates[i] - sorted_dates[i - 1]))
    diff2=$((sorted_dates[i - 1] - sorted_dates[i - 2]))

    if [ "$diff" -gt "$gap" ] && [ "$diff2" -lt "$gap" ]; then
        sdate=$(date --date @${sorted_dates[i]} +"%r")
        diff2=$((current_date - sorted_dates[i]))
        hours_ago=$(echo "scale=2; ${diff2}/3600" | bc)
        whole_hours=$(echo "(${hours_ago})/1" | bc)
        decimals=$(echo "${hours_ago}" | grep -Eo "\.[0-9]+$")
        minutes_ago=$(echo "(${decimals}*60)/1" | bc)

        if [ "$whole_hours" -eq "1" ];
        then
            shours="hour"
        else
            shours="hours"
        fi

        if [ "$minutes_ago" -eq "1" ];
        then
            sminutes="minute"
        else
            sminutes="minutes"
        fi

        message="${whole_hours} ${shours} and ${minutes_ago} ${sminutes} ago ( ${sdate} )"
        echo "$message"
        break
    fi
done

The thing with bash is more often then not... there is a tool that you can leverage to do what you want. When writing bash scripts, I try write as little bash as possible, and to leverage these tools where best possible to avoid bash (seems counter intuitive, but is the nature of bash to write as little bash as possible).  For example have you heard of the command tuptime?  (https://manpages.debian.org/testing/tuptime/tuptime.1.en.html)

This is what it looks like after a reboot (is packaged with apt):

$ sudo apt-get install tuptime
$ tuptime | grep "System downtime"
System downtime:    0.15 %   -   32 seconds

Is not a complete answer because does not cover the >5 hrs case above.

For reference, here is a run of tuptime

$ tuptime
System startups:    2   since   11:02:34 AM 02/05/2020
System shutdowns:   1 ok   <-   0 bad
System uptime:      99.85 %   -   5 hours, 56 minutes and 45 seconds
System downtime:    0.15 %   -   32 seconds
System life:        5 hours, 57 minutes and 17 seconds

Largest uptime:     5 hours, 45 minutes and 12 seconds   from   11:02:34 AM 02/05/2020
Shortest uptime:    11 minutes and 33 seconds   from   04:48:18 PM 02/05/2020
Average uptime:     2 hours, 58 minutes and 23 seconds

Largest downtime:   32 seconds   from   04:47:46 PM 02/05/2020
Shortest downtime:  32 seconds   from   04:47:46 PM 02/05/2020
Average downtime:   32 seconds

Current uptime:     11 minutes and 33 seconds   since   04:48:18 PM 02/05/2020

If wanting something more specific, I would also consider last command which gives you information about when people logged in or out (using http://man7.org/linux/man-pages/man1/last.1.html).

$ last reboot --time-format iso
reboot   system boot  4.15.0-76-generi 2020-02-05T16:48:22-0800   still running
reboot   system boot  4.15.0-76-generi 2020-02-05T11:02:38-0800 - 2020-02-05T16:47:57-0800  (05:45)
reboot   system boot  4.15.0-76-generi 2020-02-04T14:30:53-0800 - 2020-02-05T16:47:57-0800 (1+02:17)
reboot   system boot  4.15.0-76-generi 2020-02-03T15:28:38-0800 - 2020-02-04T14:30:29-0800  (23:01)
reboot   system boot  4.15.0-76-generi 2020-02-03T10:55:02-0800 - 2020-02-03T15:28:13-0800  (04:33)


Account creation and log-in system I've been learning Python for about 1 - 1 1/2 weeks now and just finished my first small project to help solidify what I've learnt.  It's a very basic (bad) login / create account system that I run in PowerShell but just wanted someone feedback and thoughts as to how I'm getting on in regards to how long I've been learning.

import os.path

def app(user_name):
    print(user_name)

def create():

    exists = os.path.exists("C:/Users/Tom/Desktop/test/login_project/account_info.txt")

    if not exists:
        f = open("account_info.txt", "w")
        f.close()

    creating_username = True
    creating_password = True

    while creating_username:
        user_name = input("\nEnter a username: ")

        f = open("account_info.txt", "r")
        file_contents = f.read()

        if f"'{user_name}'" in file_contents:
            print("\nUsername already in use.")
        else:
            creating_username = False

    email = input("Enter an e-mail address: ")

    while creating_password:

        password = input("Enter a password: ")
        password_confirm = input("Confirm password: ")

        if password == password_confirm:

            info_list = [user_name, password, email]

            f = open("account_info.txt", "a+")
            f.write(f"{info_list}\r\n")
            f.close()

            print("\nAccount Created.\n")

            creating_password = False

            choosing()

        else:
            print("\nPasswords did not match.\n")

def login():

    exists = os.path.exists("C:/Users/Tom/Desktop/test/login_project/account_info.txt")
    entering_info = True

    while entering_info:

        user_name = input("\nEnter username: ")
        password = input("Enter password: ")

        if not exists:
            entering_info = False
            print("\nError: [0] account(s) found in database\n")
            choosing()

        else:
            f = open("account_info.txt", "r")
            file_contents = f.read()

            f = open("account_info.txt", "r")
            list_ = [char for char in f.readlines() if char != "\n"]

            counter_one = 1
            counter_two = 1

            for i in list_:
                if f"'{user_name}'" in i:
                    break
                counter_one += 1

            for y in list_:
                if f"'{password}'" in y:
                    break
                counter_two += 1

            f.close()

            if f"'{user_name}'" in file_contents:
                if f"'{password}'" in file_contents and counter_one == counter_two:
                    print("Login Successful")
                    entering_info = False
                    app(user_name)
                else:
                    print("\nPassword incorrect.")
            else:
                print("\nUsername incorrect.")

def choosing():

    main_choosing = True
    main_choice = ""

    print("Please continue to login or create an account.")
    print("(Type 'login' to login, or 'create' to create an account).")
    print("('exit' to close program)\n")

    while main_choosing:

        main_choice = input("- ")

        if main_choice == "login" or main_choice == "create" or main_choice == "exit":

            if main_choice == "login":
                main_choosing = False
                login()
            elif main_choice == "create":
                main_choosing = False
                create()
            else:
                print("\nClosing...\nClosed.")
                break

        else:
            print("\nEnter a valid command.\n")

print("\nWelcome.\n")
choosing()

Welcome to code review, and good job for your first program I suppose.

Style

Docstrings: Python documentation strings (or docstrings) provide a convenient way of associating documentation with Python modules,
functions, classes, and methods. As you can see, even for a
relatively simple function, documenting using comments quickly makes
it unpleasant and difficult to read. So, to solve this, the docstring
was introduced. You should include docstrings to the functions you
created indicating what they return and some type hints if necessary.

def app(user_name):
    """Print user name."""

def create():
    """Create new account."""

Descriptive variable names: The name app(user_name) can be changed to print_username() since it's a good practice to create
descriptive variable names which improve readability for you and for
other people using your code. Same goes for create() that can be
create_account()
PEP0008 The official Python style guide https://www.python.org/dev/peps/pep-0008/ I suggest you check this, will help you write more pythonic code.
Blank lines: Your functions contain many blank lines and according to PEP0008 you should use blank lines sparingly:
Surround top-level function and class definitions with two blank
lines. Method definitions inside a class are surrounded by a single
blank line. Extra blank lines may be used (sparingly) to separate
groups of related functions. Blank lines may be omitted between a
bunch of related one-liners (e.g. a set of dummy implementations).

Code

The code is a bit long and can be shortened. Storing all usernames and passwords and everything else in the same file I think is a bad idea and is very unreliable because all things would be mixed up and you don't know which is what, I suggest you create a .txt file for each user and make the file name same as the user name so the program checks the folder and if the user is not found, it prompts the user accordingly.  And still this is not the best way to do it however it is better than mixing all things up.

exists = os.path.exists("C:/Users/Tom/Desktop/test/login_project/account_info.txt")

this line should be cleaned up or replaced by input() (If i'm using your code it's very obvious that the path will not exist and the condition if not exists: will always evaluate to True.

info_list = [user_name, password, email]

The variable user_name is referenced before assignment, you should define your variables at the top of the function body to avoid such problems.

unused variable(line 103) you should clean such things up upon completion.

main_choice = ""

Bug:
This appears after creating a new account and trying to login.
 - Error: [0] account(s) found in database

These are unecessary variables:

creating_username = True
creating_password = True

followed by while creating_username: and while_creating_password:

which can be written:

while True:
    # block

and same goes to entering_info = True and main_choosing = True in other functions.

else:
    creating_username = False

can be replaced with break in the following way:

else:
    break

instead of manually closing the file you just opened, you may use with & as context managers that automatically carry the opening and closing activities for you in the following way and the file will be closed automatically.

f = open("account_info.txt", "a+")
f.write(f"{info_list}\r\n")
f.close()

can be written:

with open('account_info.txt', 'a+') as data:
    # do things

line 69: the file was opened twice (I think you forgot to clean this up)

f = open("account_info.txt", "r")

file_contents = f.read()

f = open("account_info.txt", "r")
list_ = [char for char in f.readlines() if char != "\n"]

Descriptive variable names: I think this is the second time I mention this: a code is designed to be interpreted by machines and to be read and understood by human beings, try as much as you can to avoid ambiguous variable/method/function names.

for i in list_:
for y in list_:
counter_one = 1
counter_two = 1

what is i? what is y? what is counter_one what does it count? what is counter_two?

print("\nWelcome.\n")
choosing()

if __name__ == '__main__': guard to be used at the end of your script, allows your module to be imported by other modules without running the whole script.

This should be placed at the end of your script like the following:

if __name__ == '__main__':
    print('\nWelcome.\n')
    choosing()

I tried to simplify your program and here's what I came up with (And still this is not the proper way to do it in the real world, it's just for the sake of the example) and you might modify it if you want it to keep running and keep asking for input or whatever structure you want, you might consider this as an assignment:

import os

def create_account(path):
    """Create and save username to a Usernames folder."""
    if 'Usernames' not in os.listdir(path):
        os.mkdir('Usernames')
    os.chdir('Usernames')
    new_username = input('Enter new user name: ')
    if new_username + '.txt' in os.listdir(path + 'Usernames'):
        raise ValueError(f'Username {new_username} already exists.')
    new_password = input('Enter new password: ')
    confirm_pass = input('Confirm password: ')
    while new_password != confirm_pass:
        print('Passwords do not match!')
        new_password = input('Enter new password: ')
        confirm_pass = input('Confirm password: ')
    with open(new_username + '.txt', 'w') as new_user:
        new_user.write(new_username + ' ')
        new_user.write(new_password)
    print(f'Account created successfully.')

def login(path):
    """Login using user name and password."""
    if 'Usernames' in os.listdir(path):
        os.chdir('Usernames')
        username = input('Enter Username or q to quit: ')
        while username + '.txt' not in os.listdir(path + 'Usernames') and username != 'q':
            print(f'Username {username} does not exist.')
            username = input('Enter Username or q to quit: ')
        if username == 'q':
            exit(0)
        password = input('Enter password: ')
        if username + '.txt' in os.listdir(path + 'Usernames'):
            user, valid_pass = open(username + '.txt').read().rstrip().split()
            while password != valid_pass:
                print('Invalid password.')
                password = input('Enter password: ')
            print('Login successful.')
    else:
        print('Database not found!')
        exit(1)

def start_login(path):
    """Run login interactively."""
    print('Welcome to my program.')
    user_mode = input('Enter L for login or C for creating a new account or Q to exit.').lower()
    while user_mode not in 'lcq':
        print('Invalid command')
        user_mode = input('Enter L for login or C for creating a new account or Q to exit.').lower()
    if user_mode == 'l':
        login(path)
    if user_mode == 'c':
        create_account(path)
    if user_mode == 'q':
        print('Thank you for using my program')
        exit(0)

if __name__ == '__main__':
    database_path = input('Enter database path: ').rstrip()
    while not os.path.exists(database_path):
        print('Invalid path')
        database_path = input('Enter database path: ').rstrip()
    start_login(database_path)


Multithreaded batching class I need to write a class to batch Order objects from 3 calling components running on 3 separate threads. The 3 calling components will call the Batcher class at the same time (within a second of each other), but to handle clock issues and thread interleaving, the method should wait for a maximum of 5 seconds for the 3 calls. If for any reason only 2 calls make it, the batcher should continue after the 5-second timeout and batch what it has. The Batcher class must appear synchronized to the calling components.

I have written a class and it seems to work but I'm new to this and I'd appreciate it if someone could review my class.

public sealed class Batcher
{
    private readonly object _syncLock = new object();
    private readonly Timer _timer;
    private bool _timerSet = false;
    private volatile int _callCount = 0;
    private const int ExecuteImmediatelyCallCount = 3;
    private readonly ManualResetEvent _manualResetEvent = new ManualResetEvent(false);
    private readonly List<Order> _orders = new List<Order>();

    public Batcher()
    {
        _timer = new Timer(Batch, null, Timeout.Infinite, Timeout.Infinite);
    }

    public void  TryBatch(IEnumerable<Order> orders)
    {
        lock (_syncLock)
        {
            _orders.AddRange(orders);
            if(!_timerSet)
            {
                _timerSet = true;
                _timer.Change(5000, Timeout.Infinite);
            }
            _callCount ++;
        }

        if(_callCount >= ExecuteImmediatelyCallCount)
        {
            _timer.Change(Timeout.Infinite, Timeout.Infinite);
            Batch(null);
        }
        _manualResetEvent.WaitOne();
    }

    private void Batch(object state)
    {
        lock (_syncLock)
        {
            if(_orders.Count > 0)
            {
                RemoteService.Send(_orders);
                _orders.Clear();
                _manualResetEvent.Reset();
            }
            _callCount = 0;
        }
    }
)

Not sure what is behind the requirement of 5 seconds, but from the top level (please comment to answer the items marked as warnings):

public sealed class Batcher
    {
        private readonly object _syncLock = new object();
        private readonly Timer _timer;
        private bool _timerSet = false;
        private volatile int _callCount = 0;
        private const int ExecuteImmediatelyCallCount = 3;
        private readonly ManualResetEvent _manualResetEvent = new ManualResetEvent(false);
        private readonly List<Order> _orders = new List<Order>();

        public Batcher()
        {
            _timer = new Timer(Batch, null, Timeout.Infinite, Timeout.Infinite);
        }
#warning What is the "orders" parameter for
        public void  TryBatch(IEnumerable<Order> orders)
        {
#warning don't see a point to lock this operation
            lock (_syncLock)
            {
                _orders.AddRange(orders);
                if(!_timerSet)
                {
                    _timerSet = true;
                    _timer.Change(5000, Timeout.Infinite);
                }
                _callCount ++;
            }
#warning not thread safe
            if(_callCount >= ExecuteImmediatelyCallCount)
            {
                _timer.Change(Timeout.Infinite, Timeout.Infinite);
                Batch(null);
            }
#warning what is this for?
            _manualResetEvent.WaitOne();
        }

        private void Batch(object state)
        {
            lock (_syncLock)
            {
                if(_orders.Count > 0)
                {
#warning not thread safe
                    RemoteService.Send(_orders);
                    _orders.Clear();
                    _manualResetEvent.Reset();
                }
                _callCount = 0;
            }
        }
    )


ASP.NET Password Hashing reimplemented in PHP Live link to Code: https://github.com/sqlcollaborative/AlwaysEncryptedSample/blob/features/phpSampleApp/AlwaysEncryptedSample.PHP/src/Security/AspNetIdentity.php

Unit tests to prove it works: https://github.com/sqlcollaborative/AlwaysEncryptedSample/blob/features/phpSampleApp/AlwaysEncryptedSample.PHP/tests/Security/AspNetIdentityTest.php

I am making a php CLI apps to add users to an AlwaysEncrypted application. Here is how I am encrypting the password. Its almost a direct port of the ASP.NET C# code and has 100% code coverage so I'm pretty sure it works. Just wondering if there is a better way to do things.

<?php
declare(strict_types = 1);

namespace SqlCollaborative\AlwaysEncryptedSample\Security;

use RuntimeException;

/**
 * Class AspNetIdentity
 * @package SqlCollaborative\AlwaysEncryptedSample\Security
 * A direct port of the algorithm to generate and verify ASP.NET password hashes found here:
 * @link https://aspnetidentity.codeplex.com/SourceControl/latest#src/Microsoft.AspNet.Identity.Core/Crypto.cs
 * For mor information:
 * @link http://stackoverflow.com/questions/20621950/asp-net-identity-default-password-hasher-how-does-it-work-and-is-it-secure
 */
class AspNetIdentity
{
    const PBKDF2_ITER_COUNT = 1000; // default for Rfc2898DeriveBytes
    const PBKDF2_SUBKEY_LENGTH = 256/8; // 256 bits
    const SALT_SIZE = 128/8; // 128 bits

    private function generateRandomSalt() : string
    {
        return random_bytes(self::SALT_SIZE);
    }

    public function getPasswordSalt(string $hashedPassword) : string
    {
        $hashedPasswordByteString = base64_decode($hashedPassword);
        $expectedHashLength = 1 + self::SALT_SIZE + self::PBKDF2_SUBKEY_LENGTH;
        $actualHashLength = strlen($hashedPasswordByteString);

        $header = substr($hashedPasswordByteString, 0, 1);
        if ($header != "\0"){
            throw new RuntimeException("Incorrect header [ $header ]");
        }

        if ($actualHashLength != $expectedHashLength)
        {
            throw new RuntimeException (
                "Salted hash is wrong length [ expected = {$expectedHashLength}, actual = {$actualHashLength} ]"
            );
        }

        return substr($hashedPasswordByteString, 1, self::SALT_SIZE);
    }

    /**
     * Creates a PBKDF2 (AKA Rfc2898) hash from a plaintext password.
     * @param string $password A plaintext password
     * @param string|null $salt Optionally specify the hash. This should only be used to verify existin passwords.
     * @return string The base64 encoded password hash
     */
    public function hashPassword(string $password, string $salt = null) : string
    {
        $salt = $salt ?? $this->generateRandomSalt();
        $subkey = hash_pbkdf2(
            'sha1',  // The SHA1 exploit google discovered is irrevelevant to HMAC_SHA1
            $password,
            $salt,
            self::PBKDF2_ITER_COUNT,
            self::PBKDF2_SUBKEY_LENGTH,
            true
        );

        return base64_encode("\0" . $salt . $subkey);
    }

    /**
     * verifies a plaintext password matches its hash.
     * @param string $hashedPassword must be of the format of HashWithPassword (salt + Hash(salt+input)
     * @param string $password the plain text version of the password
     * @return bool true if the password matches false if it does not.
     */
    public function verifyHashedPassword(string $hashedPassword, string $password) : bool
    {
        try {
            $salt = $this->getPasswordSalt($hashedPassword);
        }
        catch (RuntimeException $ex) {
            trigger_error($ex->getMessage());
            return false;
        }
        $actualHashedPassword = $this->hashPassword($password, $salt);
        return ($actualHashedPassword === $hashedPassword);
    }
}

I don't have any great concern over the implementation here when thinking of it in the context of a code port.

I do have some concern that you would be continuing with a potentially bad design.

For example:

An iteration count of 1000 for PBKDF2 seems low. Most standard implementation now use 10K+, though ultimately you would need to tune this based on the hardware on which you are running the algorithm and the desired hash computation time you are looking to achieve.  You code right now gives you no way to tune this.
Which, leads my to the concern that you are not storing any information about the key length or iteration count on the hash, meaning you will never be able to "adapt" your encryption operation to keep pace with hardware changes.  You are going to be stuck with 1000 iteration computational work factor for passwords in this application until you make code changes to increase the computation time.

If possible for your implementation, I would consider adding algorithmic information (including work factor info) to your result hash such that you can adaptively change your application configuration at some point in the future without having to change code.  Say for example you decided to increase the iterations from 1000 to 10,000.  At the point of password verification, you could catch cases with outdated encryption modes and proactively regenerate the password hash to meet your new standard (of course only for cases where the password verification is successful).

Does your getPasswordSalt() method really need to be public? Is there context in which you are expecting callers to invoke this method outside of password verification?

I don't understand the need for exception being thrown by getPasswordSalt() if you are basically just swallowing it anyway in verifyHashedPassword().  I really like the idea of logging errors on expected behavior, but I would question making whether throwing/catching here is proper pattern.

I also question having an early return upon hash string validation failure. Though early exits such as this are typically a good idea, in this specific case you are potentially leaking information about the password hash (that it is invalid) to an attack who is trying to exploit the system internally.

Consider logging the unexpected input like you are now doing, but then just creating a new hash for the input password anyway even though you are going to send a false result. This way, you ensure that you always spend hash computation time during this verification step.

It would seem that the ASP equivalent does not allow caller to pass the salt.  Do you really want to allow your caller to send in an arbitrary salt to hashPassword()? Should you move the main hash generation logic into a protected/private function that can be used from inside the class, with the public interface only allowing passing of password?

For example:

public function hashPassword(string $password) : string
{
    return $this->generateHash($password);
}

private function generateHash(string $password, string $salt = null) : string
{
    // your current hashPassword() logic here
}

Should your hash string validations around first byte and overall length live on verifyHashedPassword() method instead of getPasswordSalt()? I don't know why a potentially private method like getPasswordSalt() should have validation within it. Perhaps these could even be encapsulated into their own private validateHash() method.

This would also remove the odd exception throwing/swallowing pattern mentioned before.  So you could have something like this in the verification method:

public function verifyHashedPassword(string $hashedPassword, string $password) : bool
{
    $result = $this->validateHash($hashedPassword);
    if($result === false) {
          // let's kill some time
          $this->generateHash($password);
          return false;
    }
    $salt = $this->getPasswordSalt($hashedPassword);
    $actualHashedPassword = $this->generateHash($password, $salt);
    return ($actualHashedPassword === $hashedPassword);
}

private function validateHash(string $hash) : boolean
{
    // perform validation
    // log errors if necessary
    // return boolean result
}

Even if you choose to keep getPasswordSalt() as public method, you could still reference common validation function like:

public function getPasswordSalt(string $hashedPassword) : string
{
    $result = $this->validateHash($hashedPassword);
    // early exit for validation failure
    // rest of method logic for valid hash
}

Should this be a concrete class at all?  These seem like they should be static methods. There is nothing that you are doing here that would seem to necessitate a concrete instantiation.

If you are going to use doc blocks (a good thing) you should use them uniformly across your code, not just certain methods.


Prime numbers less than a given number N So, I began this challenge on CodeEval, which asks to print out the number of prime numbers less than any given integer N, where 0 < N < 4,294,967,295, ideally in O(N log N) time.  I came up with this solution:

import java.io.*;
public class Main {
    public static void main (String[] args) throws IOException {
        File file = new File(args[0]);
        BufferedReader buffer = new BufferedReader(new FileReader(file));
        String line;
        int limit;
        while ((line = buffer.readLine()) != null) {
            StringBuilder sb = new StringBuilder();
            line = line.trim();
            limit = Integer.valueOf(line);
            for (int i = 2; i <= limit; i++)
                if (isPrime(i) || i == 2)
                    sb.append(i + ",");
            sb.deleteCharAt(sb.length() - 1);
            System.out.println(sb);
        }
    }

    private static boolean isPrime(int n) {
        if (n % 2 == 0)
            return false;
        for (int i = 3; i*i <= n; i += 2)
            if (n % i == 0)
                return false;
        return true;
    }
}

Basically, I'm building up a string where I append all prime numbers less than N (which is passed by argument). Testing it, it works perfectly (or so it seems); submitting it, I get a "partially" status and score of 85. Don't know really what I'm messing up here...

The challenge states

  Print out the prime numbers less than a given number N

but you are iterating

for (int i = 2; i <= limit; i++)

The ending condition should be i < limit.

You need to check some edge cases:

limit == 0 here you would get an exception at sb.deleteCharAt(sb.length() - 1);
limit == 1 here you would get an exception at sb.deleteCharAt(sb.length() - 1);
limit == 2 using the ending condition mentioned above you would get an exception at  at sb.deleteCharAt(sb.length() - 1); too

If you start the loop (in main)  at 3 and increment i by 2 this will be faster and you wouldn't need if (n % 2 == 0)
Using braces {} for single if statements and loop bodies will make your code less errorprone.
importing the whole java.io namespace is to much. Restrict the imports to the classes you need.
the StringBuilder.append() method returns a StringBuilder object. So instead of calling sb.append(i + ",") you should call sb.append(i).append(",")

Refactoring

The while loop

while ((line = buffer.readLine()) != null) {
    StringBuilder sb = new StringBuilder();
    line = line.trim();
    limit = Integer.valueOf(line);
    if (limit > 2) {
         sb.append(2).append(",");
    }
    for (int i = 3; i < limit; i += 2){
        if (isPrime(i)){
            sb.append(i).append(",");
        }
    }
    if (sb.length() > 0) {
        sb.deleteCharAt(sb.length() - 1);
    }
    System.out.println(sb);
}

and the isPrime() method

private static boolean isPrime(int n) {
    for (int i = 3; i*i <= n; i += 2) {
        if (n % i == 0) {
            return false;
        }
    }
    return true;
}

in this way isPrime() would return true for each int n < 3 but this is for this case perfectly fine, because this is a private method which is only called in this specific szenario given strong borders.


Find anagrams of a word in a list I'm just starting to learn programming again after a long break and decided on Python. Here's my code:

#"Code Wars: Where my anagrams at?" A program that finds all anagrams of "word" in "wordList".

import copy

def anagrams(word, wordList):
    result = [] #List to store all the anagrams.
    for i in wordList:  #Outer loop finds all anagrams of "word".
        source = copy.deepcopy(list(i))   #Creates a copy of the current word in the word list as a list.
        var = True #Boolean value that indicates whether "i" is an anagram of "word" or not.
        for j in word:  #Inner loop. It checks if "i" is an anagram of "word".
            if(j in source):    #Incrementally deletes the matching characters.
                source.remove(j)
            else:   #Early exit if a single character in "word" is missing from "i".
                var = False
                break
        if(source != []):    #If "i" is an anagram of "word", then "source" should be empty.
            var = False
        if(var):   #Add the word to the result list iff "i" is an anagram of "word".
            result.append(i)
    return(result)

What am I doing wrong (re best practices/design/efficiency)?
What could I do better?

The most pythonic way (IMO) to check if two words are anagrams of each other is to check if the number of times each letter appears in each words is the same. For this you can use collections.Counter:

from collections import Counter

>>> Counter("test")
Counter({'e': 1, 's': 1, 't': 2})

With this we can re-write your function to :

def anagrams(word, words):
    result = []
    counter_word = Counter(word)
    for other_word in words:
        if Counter(other_word) == counter_word:
            result.append(other_word)
    return result

This is a lot more readable. in addition, here are some more comments on your code itself:

source = copy.deepcopy(list(i)) is not needed. Since strings are immutable, just doing list(i) would have been fine.
In if(j in source): there are two things:

Python has an official style-guide, PEP8, which is well worth a read. It recommends not using parenthesis in if (and while) statements, so this should be if j in source
in is \$\mathcal{O}(n)\$ for lists and strings, so this is actually quite an expensive way to do it. The better way is using a dictionary (collections.Counter is a dictionary plus some nice stuff).

In for i in wordList, again, PEP8 has something to say, it should be word_list, or even easier words. Also i is not a good name.

Depending on what you want to do afterwards with the anagrams, you could also make this a generator (which you can iterate over and which continuesly yields anagrams):

def anagrams(word, words):
    counter_word = Counter(word)
    for other_word in words:
        if Counter(other_word) == counter_word:
            yield other_word


Simple twin prime finder I'm really new to coding so I'm looking for a bit of feedback on this twin prime finder. She works pretty decent but I know it could be better.

import math
primes = []
def get_primes(lower, upper):
  for num in range(lower, upper):
    for i in range(2, num):
      if num % i == 0:
        break
    else:
      primes.append(num)
print get_primes(int(raw_input("Enter lower boundary")), int(raw_input("Enter upper boundary")))
print primes

for i in range(0, len(primes) - 1):
  if primes[i+1] - primes[i] == 2:
    print str(primes[i]), str(primes[i+1])

Style

The Style Guide for Python Code called PEP 8 recommends a 4-space indent.

More functions

You could split your logic into smaller logical pieces which are easier to understand, to test and to optimise (I'll come back to this later).

For instance:

def is_prime(n):
    for i in range(2, n):
        if n % i == 0:
            return False
    return True

def get_primes(lower, upper):
    for num in range(lower, upper):
        if is_prime(num):
            primes.append(num)

Avoid global

Your function get_primes would be much easier to use if instead of appending to a global variable, it was using a proper return value.

def get_primes(lower, upper):
    primes = []
    for num in range(lower, upper):
        if is_prime(num):
            primes.append(num)
    return primes

primes = get_primes(5, 56)  # get_primes(500000, 560000)
print primes

More functions (again)

You could add a function get_twin_primes to return ... the twin primes.

if __name__ == "__main__":

It is practical (and conventional) in Python to use the if __name__ == "__main__": to separate your functions/classes definitions from the code actually using it. This makes code reuse possible via import : the functions/classes can be used by other modules without triggering all your computations.

Now, the whole code looks like:

import math

def is_prime(n):
    for i in range(2, n):
        if n % i == 0:
            return False
    return True

def get_primes(lower, upper):
    primes = []
    for num in range(lower, upper):
        if is_prime(num):
            primes.append(num)
    return primes

def get_twin_primes(primes):
    twins = []
    for i in range(0, len(primes) - 1):
        if primes[i+1] - primes[i] == 2:
            twins.append((primes[i], primes[i+1]))
    return twins

if __name__ == "__main__":
    primes = get_primes(5, 56)  # get_primes(500000, 560000)
    print primes
    for a, b in get_twin_primes(primes):
        print a, b

More beautiful loop

I highly recommend Ned Batchelder's excellent talk called "Loop Like A Native" (and any other talk he's done). You'll learn a lot about iterations in Python and that basically, anytime you write "range(len(list))", there is a better way.

In your case, you'll find various ways to iterate over consecutive items in a list like this:

def get_twin_primes(primes):
    twins = []
    for p1, p2 in zip(primes, primes[1:]):
        if p2 - p1 == 2:
            twins.append((p1, p2))
    return twins

(In Python 2, you could use itertools.izip instead of zip but if you're learning Python, you should try to use Python 3 directly).

Then, this can easily be rewritten using a list comprehension:

def get_twin_primes(primes):
    return [(p1, p2) for p1, p2 in zip(primes, primes[1:]) if p2 - p1 == 2]

More list comprehensions

You can also use list comprehensions in get_primes:

def get_primes(lower, upper):
    return [num for num in range(lower, upper) if is_prime(num)]

Using builtins (and generator expressions)

Your function could be writen in a more concise way using the builtin all.

def is_prime(n):
    return all(n % i != 0 for i in range(2, n))

You can easily read this as "a number is prime is all divisions have a non-zero remainder".

It's exactly like the logic you had before but you are reusing already existing builtins and it teaches you new things :)

Now, the whole code looks like:

import math

def is_prime(n):
    return all(n % i != 0 for i in range(2, n))

def get_primes(lower, upper):
    return [num for num in range(lower, upper) if is_prime(num)]

def get_twin_primes(primes):
    return [(p1, p2) for p1, p2 in zip(primes, primes[1:]) if p2 - p1 == 2]

if __name__ == "__main__":
    primes = get_primes(5, 56)  # get_primes(500000, 560000)
    print primes
    for a, b in get_twin_primes(primes):
        print a, b

Optimisation

A simple yet very efficient optimisation when checking for prime is to use the fact that if a number is composite, for any pair of factors, one is less than or equal to the square root, and the other is greater than or equal to the square root, so you don't need to check the values larger than the square root of the number you are considering. There you can write something like:

def is_prime(n):
    """Checks if a number is prime."""
    if n < 2:
        return False
    return all(n % i for i in range(2, int(math.sqrt(n)) + 1))

Another optimisation you could do is to use the step argument of range in get_primes to iterate over odd numbers only (and add "2" at the beginning if needed).


Passing argument to a constructor and using it only in some class methods I've created some controller for handling clicked links statistics. Does this class meet Single Responsibility Principle?

class StatisticsController
{
    protected $statisticsQuery;

    public function __construct(StatisticsQuery $statisticsQuery)
    {
        $this->statisticsQuery = $statisticsQuery;
    }

    public function recordClickedLink(array $request)
    {
        (...)
        if ($this->canRecordClick()) {
            // record clicked link
        }
        (...)
    }

    public function getStatistics(array $request)
    {
        (...)
        $someRequestParam = $request['id'];
        $statistics = $this->statisticsQuery->get($someRequestParam);
        (...)
    }

    protected function canRecordClick()
    {
        // returns true of false
    }
}

If has two public methods. The first is responsible for saving clicked link to a database by delegating it to a model class. The second public method is responsible for getting statistics and it is the only method which use $statisticsQuery object passed to a class' constructor. Is it OK that recordClickedLink method doesn't need constructor's argument?

I wonder also if it is OK that my class contain canRecordClick method with logic for checking if link click can be saved during particular request.

If this class doesn't meet SRP, how can I refactor it?

As far as I can see in the code example you've provided, it seems that the getStatistics and recordClickedLink methods are independent of each other. So it seems intuitive to have in this case two controllers: a StatisticsController and a something like a RecordController where the two methods recordClickedLink and canRecordClick live. In that way the StatisticsController is responsible for statisics and the RecordController for the record.

It is ok that the class contains canRecordClick, but protected means that any class extending the StatisticsController will be able to call this method too. If it will only be used in the StatisticsController, private would be better.


What is the best way to get the child class name for use in a parent class I am currently trying to implement a very simple MVC framework as a way of helping me understand how they work. My base controller class has 2 properties, a model and a view. All controllers have a generic view class property however only certain controllers have a model.

I needed a way of obtaining the name of the child controller class in the base controller class so that in the constructor I can check to see if a model class exists and instantiate one if so. I think I have come up with a solution using late static binding. I do not fully understand the concept, but as far as I can tell, if used in a parent class the static keyword refers to the child class.

I also implemented a function className() in the controller class which uses the 'this' keyword to get the name of the current class. This function is then called with the static keyword resulting in the name of the child class being obtained.

Below is the code for the Controller class. All models are titled controllerName_model.

/**
* Base controller class, creates a generic view class and a model class (if one exists for the child class) for the controller.
*/
class Controller
{
    public $view;
    public $model;

    public function __construct()
    {
        //Create a new genereic view object
        $this->view = new View();

        //Use late static binding to get the name of the child class extending this base class
        $name = static::className();
        //Create the path to the related model (if it exists)
        $path = 'models/' . $name . '_model.php';

        //If the model exists then require the file and create a new model object and store in $this->model
        if(file_exists($path)) {
            require $path;

            $modelName = $name . '_Model';
            $this->model = new $modelName();
        }
    }

    /**
    * This class returns the name of each class that extends this base controller class. It is used in the
    * constructor with late static binding to get the name of the child class so that a model can be created if one exists.
    */
    public function className() {
        return get_class($this);
    }
}

As far as I can tell this method is working but I am not confident that it is correct or that it is the best way of doing this. If anyone knows of any other ways of doing this or can point out any flaws in this approach I would be extremely grateful. Thanks.

Looks fine to me; though somethings to think about. Do you need to run get_class? Or might it be better to have a model variable in the child class

class Blog extends Controller
{
    private $modelClass = 'Blog';
}

The overhead of get_class might be small, but probably more than a variable; but this way also give you the ability of being able to use the same model with multiple controllers if you need to. Granted you could just inherit with the models too, but that would mean 2 includes instead of one.

If you don't go that route, consider some validation, string tidying or consistency in your model/file names.

$path = 'models/' . $name . '_model.php';

will likely equal Blog_model.php but your class is Blog_Model see the mixing of cases; consider Blog_Model.php or doing a strtolower($name). This might seem minor, but different Operating Systems handle case differently and I have been bitten by this exact issue.

I noticed you are doing a require rather than a require_once which could lead you to have cannot re-declare class errors. if require_once isn't for you consider

if(!class_exists($modelName))

This might seems abstract at the moment, but when I came to upgrade an admin system for a custom CMS I found it really handy for the admin Controllers to wrap the frontend wesbites controllers, and adding a little checking could greatly increase your flexibility further down the road.

If you haven't done already check out __autoload http://php.net/manual/en/language.oop5.autoload.php if might be more overhead, but it is just so handy I really miss it on legacy projects.


C++ breadth-first search using adjacency matrix I have written this Breadth First Search program using Adjacency Matrix, taking help from Introduction to Algorithm(CLSR) and from Internet.

I want to optimise this code.

#include <iostream>
#include <queue>
#include <limits>
#include <vector>

class Graph
{
  int vertexCount;
  //WHITE means Undiscovered, GRAY means Discovered, BLACK menas Processed
  enum class Color{ WHITE, GRAY, BLACK };

  const int imax = std::numeric_limits<int>::max();

  struct Vertex
  {
    int id;
    Color color;
    size_t distance;

    Vertex(const int vertex, Color clr, int imax) : id(vertex),
                                                    color(clr),
                                                    distance(imax)
                                                    {};
  };
  std::vector< std::vector<bool> > adjMatrix; //adjacency adjMatrix
  std::vector<Vertex> vertices;
public:
    Graph(int size)
  {
    vertexCount = size;
    adjMatrix.resize(vertexCount, std::vector<bool>(vertexCount));
    for(int i = 0; i < vertexCount; i++)
    {
      vertices.push_back( Vertex(i, Color::WHITE, imax));
      for(int j = 0; j < vertexCount; j++)
        adjMatrix[i][j] = false;
    }
  }
    ~Graph() {};

    void BFS(const int);
    void addEdge(int, int);
    void printPath(const int,  const int) const;

};

void Graph::BFS(const int src)
{
    const auto s = vertices[src].id;
    vertices[s].color = Color::GRAY;
    vertices[s].distance = 0;

    std::queue<int> Q;
    Q.push(vertices[s].id);

    while(!Q.empty())
    {
        auto u = Q.front();
        Q.pop();

        for (int j = 0; j < vertexCount; j++)
        {
            if(vertices[j].color == Color::WHITE && adjMatrix[u][j] == true)
            {
                vertices[j].color = Color::GRAY;
                vertices[j].distance = vertices[u].distance + 1;
                Q.push(j);
            }
        }
        vertices[u].color = Color::BLACK;
    }
}

void Graph::addEdge(int u, int v)
{
    adjMatrix[u][v] = true;
  adjMatrix[v][u] = true;
}

void Graph::printPath(const int src, const int ver) const
{
    auto s = vertices[src].id;
    auto v = vertices[ver].id;

  std::cout << s;
  for(int j = s + 1; j <= v; j++)
  {
    if(adjMatrix[s][j] == true)
    {
      std::cout << " --> "<< j;
      s = j;
    }
  }
}

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

    grp1.BFS(0);
    grp1.printPath(1, 7);
}

A bug

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

Advice 1

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

Advice 2

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

Advice 3

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

Advice 4

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

Advice 5

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

Advice 6

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

Alternative implementation

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

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

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

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

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

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

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

        if (current_node == source)
        {
            break;
        }

        current_node = parent_map[current_node];
    }

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

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

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

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

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

    return parent_map;
}

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

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

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

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


LINQish command line parser I needed to parse some command line switches from a string and I tried to do it the easy way so I just wrote this:

static class CommandLineParser
{
    public static IEnumerable<string> ParseCommandLine(this string text)
    {
        if (Regex.Matches(text, "\"").Count % 2 != 0)
        {
            throw new ArgumentException("Invalid number of qotes.");
        }
        return text.Aggregate(new
        {
            lastChar = '\0',
            isEscaped = false,
            result = new List<StringBuilder> { new StringBuilder() }
        }, (state, next) =>
        {
            var isQuote = next == '"';
            var isEscapedQuote = state.lastChar == '\\' && isQuote;

            if (next == ' ' && !state.isEscaped)
            {
                // Ignore multiple spaces between switches.
                if (state.lastChar != ' ')
                {
                    state.result.Add(new StringBuilder());
                }
            }
            else
            {
                state.result.Last().Append(next);
            }

            return new
            {
                lastChar = next,
                isEscaped = isQuote && !isEscapedQuote ? !state.isEscaped : state.isEscaped,
                result = state.result
            };
        }).result.Select(x => x.ToString());
    }
}

Example:

var text = "foo -bar -baz=\"abc \\\"def\\\"   ghi\"    -qux";
text.ParseCommandLine().Dump();

and the result:

foo
-bar
-baz="abc \"def\" ghi"
-qux

What do you think? Is this enough? Can this be even shorter?

Validate your text argument. This causes an ArgumentNullException in Regex.Matches:

((string)null).ParseCommandLine();

It would be better to check the argument before passing it on:

if (text == null)
{
    throw new ArgumentNullException(nameof(text), "Command line arguments cannot be null");
}

Why is this better? The stack trace is more meaningful because the error is thrown closer to where the problem call is and you can supply your own context specific error message. Either that, or you can return an empty array for null.

Properties should be PascalCase - including anonymous types. lastChar should be LastChar and the same for the rest.

result should be plural as it is a collection: Results.

Use default() to get the default value, it signals intent better than a literal ('\0' == default(char)).

Name your constants! E.g. the double quote and the space. A single space is the most difficult thing to read in source code (in my experience).

I think this is good code (all the above is pretty minor). I'd never thought of using Aggregate over a string for simple parsing before :)


Loading interstitial adMob in different threads I want to load Interstitial AdMob after 5 second, after the Activity started, in another Thread. Is this code right, or I'm duplicating Runnables? Is there a better way?

    Handler handler;

@Override
protected void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);
    setActivityImmersiveMode();
    setContentView(R.layout.activity_normal_mode);

    // loading ads
    handler = new Handler();

    Runnable runnable = new Runnable() {
        @Override
        public void run() {

            handler.postDelayed(new Runnable() {
                @Override
                public void run() {
                    loadInterstitialAd();
                }
            }, 5000);

        }
    };

    Thread thread = new Thread(runnable);
    thread.setName("AdThread");
    thread.start();

}

There is no need to use Thread class you can directly do this:

// loading ads
    handler = new Handler();

            handler.postDelayed(new Runnable() {
                @Override
                public void run() {
                    loadInterstitialAd();
                }
            }, 5000);

So what is happening your handler will schedule your Runnable after 5 sec, you don't have to instanciate Thread and call runnable again.


Find all possible words in a board of characters This question has 2 possible solutions and I am trying to implement the DFS solution.

Here is the Trie and DFS solution: Boggle using Trie and DFS

Please review for performance.

  Given a dictionary, a method to do lookup in dictionary and a M x N
  board where every cell has one character. Find all possible words that
  can be formed by a sequence of adjacent characters. Note that we can
  move to any of 8 adjacent characters, but a word should not have
  multiple instances of same cell.

Example:

Input: dictionary[] = {"GEEKS", "FOR", "QUIZ", "GO"};
       boggle[][]   = {{'G', 'I', 'Z'},
                       {'U', 'E', 'K'},
                       {'Q', 'S', 'E'}};
      isWord(str): returns true if str is present in dictionary
                   else false.

Output:  Following words of dictionary are present
         GEEKS
         QUIZ

using System;
using System.Collections.Generic;
using System.Text;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace GraphsQuestions
{
    /// <summary>
    /// https://www.geeksforgeeks.org/boggle-find-possible-words-board-characters/
    /// </summary>
    [TestClass]
    public class BoggleDfs
    {
        private List<string> _list = new List<string>();
        [TestMethod]
        public void GeeksForGeeksTest()
        {
            string[] dictionary = { "GEEKS", "FOR", "QUIZ", "GO" };
            char[,] boggle = {{'G', 'I', 'Z'},
                              {'U', 'E', 'K'},
                              {'Q', 'S', 'E'}};

            FindWords(boggle, dictionary);
            string[] expected = { "GEEKS", "QUIZ" };
            CollectionAssert.AreEqual(expected,_list.ToArray());
        }

        private void FindWords(char[,] boggle, string[] dictionary)
        {
            bool[,] visited = new bool[boggle.GetLength(0), boggle.GetLength(1)];
            StringBuilder str = new StringBuilder();
            //run DFS for all the options and compare with the dictionary
            for (int i = 0; i < boggle.GetLength(0); i++)
            {
                for (int j = 0; j < boggle.GetLength(1); j++)
                {
                    DFS(i, j, boggle, dictionary, str, visited);
                }
            }
        }

        private void DFS(int i, int j, char[,] boggle, string[] dictionary, StringBuilder str, bool[,] visited)
        {
            //mark we already visited this vertex
            visited[i, j] = true;

            str.Append(boggle[i, j]);
            if (IsWord(str.ToString(), dictionary))
            {
                _list.Add(str.ToString());
            }

            for (int row = i - 1; row <= i + 1 && row < boggle.GetLength(0); row++)
            {
                for (int col = j - 1; col <= j + 1 && col < boggle.GetLength(1); col++)
                {
                    if (col >= 0 && row >= 0 && !visited[row, col])
                    {
                        DFS(row, col, boggle, dictionary, str, visited);
                    }
                }
            }

            visited[i, j] = false;
            str.Remove(str.Length - 1, 1);
        }

        private bool IsWord(string str, string[] dictionary)
        {
            for (int i = 0; i < dictionary.Length; i++)
            {
                if (string.CompareOrdinal(str, dictionary[i]) == 0)
                {
                    return true;
                }
            }
            return false;
        }
    }
}

If I were you, I  would stop having the tests mixed up with the implementation. It is somehow "smart", and for small projects like this, you can say it doesn't matter. But IMO it does matter because it prevents you from thinking properly about the design of you code.

Your implementation is a nearly one-to-one implementation of the C++ from Geeks and it appears rather C++-ish.

When it comes to naming, you should call your members after what they do or represent, - not after how they do it or is: DFS() as well as _list tell me nothing.

        for (int row = i - 1; row <= i + 1 && row < boggle.GetLength(0); row++)
        {
            for (int col = j - 1; col <= j + 1 && col < boggle.GetLength(1); col++)
            {
                if (col >= 0 && row >= 0 && !visited[row, col])
                {
                    DFS(row, col, boggle, dictionary, str, visited);
                }
            }
        }

IMO this way to determine the boundaries of the loops is hard to follow. The stop conditions are calculated in the for-statements while the start conditions are determined half in the for-statements and half in the if-statement.

A more clean and C#-ish way could be something like:

  int minRow = Math.Max(0, row - 1);
  int maxRow = Math.Min(rows, row + 2);
  int minCol = Math.Max(0, col - 1);
  int maxCol = Math.Min(cols, col + 2);

  for (int r = minRow; r < maxRow; r++)
  {
    for (int c = minCol; c < maxCol; c++)
    {
      if (!visited[r, c])
        Search(r, c);
    }
  }

The conditions are calculated once before the loops and hence separated nicely from the loops,  and the loops are immediately easy to read and understand. For rows and cols see below.

    private bool IsWord(string str, string[] dictionary)
    {
        for (int i = 0; i < dictionary.Length; i++)
        {
            if (string.CompareOrdinal(str, dictionary[i]) == 0)
            {
                return true;
            }
        }
        return false;
    }

In C# this is a one-liner:

    private bool IsWord(string str, string[] dictionary)
    {
        return dictionary.Contains(str);
    }

        str.Remove(str.Length - 1, 1);

On the StringBuilder it is possible to set the length, which might be faster than removing a sequence of the one last character:

str.Length -= 1;

About the overall design:

On the one hand you let _list be a class member, while all other variables are arguments to the methods. I think I would make a statefull object having as many of the variables as possible as members:

  class WordFinder
  {
    private readonly char[,] boggle;
    private readonly bool[,] visited;
    private readonly int rows;
    private readonly int cols;
    private readonly List<string> words = new List<string>();
    private readonly StringBuilder word = new StringBuilder();
    private string[] dictionary;

    public WordFinder(char[,] boggle)
    {
      this.boggle = boggle;
      rows = boggle.GetLength(0);
      cols = boggle.GetLength(1);
      visited = new bool[rows, cols];
    }
    ...

Other than a reduced set of arguments for the methods one benefit is that the calculation of the boggle dimensions is done once here.

The FindWords method could then look like:

public IList<string> FindWords(string[] dictionary)
{
  this.dictionary = dictionary;
  words.Clear();
  word.Clear();

  for (int row = 0; row < rows; row++)
  {
    for (int col = 0; col < cols; col++)
    {
      Search(row, col);
    }
  }

  return words;
}

where the Search() method is:

private void Search(int row, int col)
{
  visited[row, col] = true;

  word.Append(boggle[row, col]);
  AddIfWord(word.ToString());

  int minRow = Math.Max(0, row - 1);
  int maxRow = Math.Min(rows, row + 2);
  int minCol = Math.Max(0, col - 1);
  int maxCol = Math.Min(cols, col + 2);

  for (int r = minRow; r < maxRow; r++)
  {
    for (int c = minCol; c < maxCol; c++)
    {
      if (!visited[r, c])
        Search(r, c);
    }
  }

  word.Length -= 1;
  visited[row, col] = false;
}

and AddIfWord() is

private void AddIfWord(string candidate)
{
  if (dictionary.Contains(candidate))
    words.Add(candidate);
}

Maybe word should be an argument rather than a member, because it is somehow a "local" variable, but on the other hand there is only one word at the time while the process is running, so it's fairly safe to let it be a member.

For convenience you could provide a static member as:

public static IList<string> FindWords(char[,] boggle, string[] dictionary)
{
  WordFinder finder = new WordFinder(boggle);
  return finder.FindWords(dictionary);
}

We could argue about if boggle and dictionary should be arguments to the constructor or FindWords() - that may be a matter of - if the process is about finding words from dictionary in boggle or visa versa and/or a matter of habit and taste.

When it comes to performance, there is not much to say, because the algorithm is "optimized" by its definition - I can't see any other ways to implement this implementation so to speak.


Loading Files from a Directory in Haskell I'm a very new Haskell programmer, and I've written the following code for loading TSV files. I think it can be improved, but I'm not sure how.

I feel like the giant "do" block in loadData is inelegant, and that there's likely a better way to approach this.
I think I'm trying to avoid the IO monad because I'm not sure how to work with it, but that there's probably a better way to handle mapping parseTSV over the contents of the files.

One note: I'm not super concerned about performance - this will run once at the beginning of my program. I want to load in all the files entirely to build a composite data structure from their contents.

module LanguageMachine (loadData) where

import System.Directory (listDirectory)
import Data.List ((\\), elemIndex)
import Data.Maybe (mapMaybe)

parseTsv :: String -> [(String, Int)]
parseTsv contents = mapMaybe parseLine (lines contents)

parseLine :: String -> Maybe (String, Int)
parseLine line =
    case elemIndex '\t' line of
        Just i ->
            let
                (word, count) = splitAt i line
            in
                Just (word, read count :: Int)
        Nothing -> Nothing

loadData :: FilePath -> [FilePath] -> IO [(String, [(String, Int)])]
loadData path exclude = do
    files <- listDirectory path
    let filtered = files \\ exclude
    let prefixed = map ((path ++ "/") ++) filtered
    contents <- traverse readFile prefixed
    let results = map parseTsv contents
    return $ zip filtered results

The files look like this, which are two-value TSV lines of a word and then the number of occurrences of that word:

ARIA    4308
ORE 4208

Thank you!

The loadData function looks just fine to me. I don't see a way it could be any more elegant given what it's doing. I do actually like the way you have separated reading the file and parsing it. That's the way to go.

Your concern about "avoiding the IO monad" shouldn't be a concern. On the contrary, that's actually a "best practice", known as "pushing I/O to the boundary". It means that the majority of your program should be pure, and only a thin shell should deal with external world. This way you can easily test and debug the majority of your program.

The only thing I might do is make the last three lines one expression:

traverse readFile prefixed
  <&> map parseTsv
  <&> zip filtered

I find this a bit more readable, but that's just a matter of personal taste.

But the parseLine function could sure use some improvement.

First, the control flow. Look what it's doing: compute a Maybe value, then if it's Just, compute another Maybe value from its contents. That's exactly what "bind" does! This means it can be nicely represented as a do block.

Second, read is a partial function. This means it can fail at runtime if there is a non-number in the file. I understand this probably Should Never Happen™, but you'd be surprised how many things that "should never happen" do, in fact, happen all the time. :-)

But since you're already in the Maybe context, it would be easy to use readMaybe instead, and skip the malformed numbers.

Applying both points:

parseLine line = do
    i <- elemIndex '\t' line
    let (word, strCount) = splitAt i line
    count <- readMaybe strCount
    return (word, count)


Check whether an array can be the result of merging two smaller arrays, maintaining order I'm prepping for a tech interview coming up and trying to have a better understanding of what I should look out for in writing code. Also, feel free to correct the terminology that I use as I want to be able to talk about my code clearly and succinctly.

One of the specifications for the tech interview process is that I use idiomatic JavaScript, according to the recruiter. I'm not familiar with that term and would appreciate any feedback on how to best write to that standard. Is there a specific standard that I should adhere to? Is that related to proper naming conventions?

Is there a better way for me to optimize time and space in my code?

When I assign values to variables within loops, are there rules that I should be aware of for when I shouldn't? For example, the current element being iterated over assigned to currentH1 variable, in my mind, makes it more readable in understanding what's happening as opposed to half1[h1Index], and I reuse it more than once. Plus, I believe this might make it more idiomatic but I'm not sure? Am I losing out on space complexity in any way? Or is there something that I may not be aware of by checking current against undefined instead of checking the current index against the size of the array?

When I assign values to variables outside of loops but within the function, are there space complexities that I should pay attention to or does garbage collection take care of this? Is this \$O(1)\$ space complexity as I'm just keeping track of the indices? Feel free to breakdown space complexity as I truly do want to have a more solid understanding of memory management.

I believe I've accounted for the edge cases that I can think of, but are there more that I should be aware of?

I placed the if condition for checking lengths at the top even before the index definitions because I figured that if they aren't even the same size, why bother with doing anything else. Is that weird?

function isMergedArray(half1, half2, mergedArray) {
  if ((half1.length + half2.length) != mergedArray.length ) {
    return false;
  }

  let h1Index = half1.length - 1;
  let h2Index = half2.length - 1;
  let maIndex = mergedArray.length - 1;

  while (maIndex >= 0) {
    const currentH1 = half1[h1Index];
    const currentH2 = half2[h2Index];
    const currentMa = mergedArray[maIndex];

    if (currentH1 != undefined && currentH1 === currentMa) {
      h1Index--;
    } else if (currentH2 != undefined && currentH2 === currentMa) {
      h2Index--;
    } else {
      return false;
    }

    maIndex--;
  }

  return true;
}

I placed the if condition for checking lengths at the top even before the index definitions because I figured that if they aren't even the same size, why bother with doing anything else. Is that weird?

No I don't think that is weird. I would consider it a good optimization.

Personally I think the code is mostly fine, a good starting point would be to Google some examples on how to merge sub-arrays since that is fairly similar.

My one question would be why you started at the end of the array and merged them in descending order. That is kind of strange to me. I would do it in ascending order. I would also use a for loop instead of a while loop.


Checking whether four text fields are all empty I have a code which checks if any of the 4 textfields on our server contains text. If even one of the four textboxes is not empty, the generated table won´t skip that row and will draw it, otherwise the row will be completely skipped and not shown in the table.

For that I declared the variables for the words and the Boolean, so I can later use it to hide the table row. What the code does is it checks all the possible combinations of the wordboxes and if even one on the wordboxes contains something, the checkRow variable  will be true.

This "if" statement method that I use works, however it is very hard to follow and I think it could be better done. I have documented myself but all the alternatives involve jquery - which is not supported on our webclient.
Does anyone have a suggestion how to better this code?

                var checkRow=false;
                var word1='{{WORD1}}';
                var word2 ='{{WORD2}}';
                var word3 ='{{WORD3}}';
                var word4 ='{{WORD4}}';
                if ((word1 !=''&& word2 !=''&& word3 !=''&& word4 != '') ||(word1 ==''&& word2 ==''&& word3 !='' && word4!='') || (word1 ==''&& word2 ==''&& word3 =='' && word4!='')
                || (word1 ==''&& word2 !=''&& word3 !='' && word4!='') || (word1 !=''&& word2 !=''&& word3 =='' && word4=='') || (word1 !=''&& word2 ==''&& word3 =='' && word4!='')
                || (word1 !=''&& word2 ==''&& word3 !='' && word4 =='') || (word1 ==''&& word2 !=''&& word3 !='' && word4=='') || (word1 ==''&& word2 !=''&& word3 =='' && word4!='')
                || (word1 !=''&& word2 ==''&& word3 =='' && word4 =='') || (word1 ==''&& word2 !=''&& word3 =='' && word4=='') || (word1 ==''&& word2 ==''&& word3 !='' && word4!='')) {
                    var checkRow= true;
                } else
                var checkRow= false;

you could just say

var checkRow = false;
if(word1 != '' || word2 != '' || word3 != '' || word4 != '') {
    checkRow = true;

or you could concat the single words and check if the length is greater than 0

EDIT:
since some people favored the string-concat solution: actually that was kind of a joke in the sense of https://en.wikipedia.org/wiki/Barometer_question. if the strings are not large it doesn't make much of a difference, but from a performance-point of view that would not be a good solution.


Project Euler #2 in F# Project Euler Problem 2 asks for the sum of all even Fibonacci numbers below 4 million.

My first attempt at this problem using functional programming with F#. I would've liked to use some kind of take-while function to keep generating Fibonacci numbers until they fulfilled certain conditions, but alas I could not find such a thing in F#.

open System

let rec fib n = match n with
                | 0 | 1 | 2 -> n
                | _ -> fib (n - 1) + fib (n - 2)

let rec nOfFibTermsUpUntil x y = if fib (y + 1) < x then nOfFibTermsUpUntil x (y + 1) else y

[for n in 0..nOfFibTermsUpUntil 4000000 0 -> fib n]
|> List.filter (fun x -> x % 2 = 0)
|> List.sum
|> printf "The sum of Fibonacci terms up until 4,000,000 is: %d"
Console.ReadKey () |> ignore

I discovered an (arguably) better way of doing this in F# using Seq.unfold and tuples. I'll post the solution below.

open System

(1,1) |> Seq.unfold (fun (a, b) -> Some(a + b, (b, a + b)))
|> Seq.takeWhile (fun x -> x <= 4000000)
|> Seq.sumBy (fun x -> if x % 2 = 0 then x else 0)
|> printf "The sum of Fibonacci terms up until 4,000,000 is: %d"
Console.ReadKey () |> ignore