C++ vs D - Algorithm Optimization/Conversion (Using vectors/arrays) OK, I've been playing with D for a while (and been in love with its expressive power and simplicity, to be honest). However, since I'm still new to D, I'm facing a few issues.

Let's take the following example. All the following program does is to take numbers in the 0..10000000 (fairly big one, for benchmarking purposes) and for each one of them return a vector/array with the positions of bits set (in binary).

E.g.

4 = 100(2) => [ 2 ]
5 = 101(2) => [ 0, 2 ]
6 = 110(2) => [ 1, 2 ]
7 = 111(2) => [ 0, 1, 2]

And so on...

Now here's my C++ code (no vector reserve etc being used) :

// bits.cpp

#include <vector>
#include <iostream>
#include "math.h"

using namespace std;

vector<unsigned int> bitsSet(unsigned long long bitboard)
{
    unsigned int n;
    vector<unsigned int> res;

    for (n = 0; bitboard != 0; n++, bitboard &= (bitboard - 1))
    {
        res.push_back(log2(bitboard & ~(bitboard-1)));
    }

    return res;
}

int main()
{
    for (int k=0; k<10000000; k++)
    {
        vector<unsigned int> res = bitsSet(k);
    }

    return 0;
}

And here's my D code :

// bits.d

import std.stdio;
import std.math;

int[] bitsSet(ulong bitboard)
{
    int[] res;

    for (int n=0; bitboard!=0; n++, bitboard&=(bitboard-1))
        res ~= cast(int)log2(bitboard & ~(bitboard-1));

    return res;
}

void main(string[] args)
{
    for (ulong k=0; k<10000000; k++)
    {
        int[] bits = bitsSet(k);
    }
}

Now, given that the 2 pieces of code are compiled with g++ bits.cpp -o cbits (or clang++ bits.cpp -o cbits) and dmd bits.d -ofdbits, respectively, these are my benchmark results, using time (on Mac OS X 10.8.2) :

For C++ :

time ./cbits

real    0m19.742s
user    0m19.722s
sys 0m0.012s

For D :

time ./dbits

real    0m14.914s
user    0m14.891s
sys 0m0.017s

This looks OK. (with D being - for me - noticeably faster).

NOTE :

Now, if I try to use something like res.reserve(64); in my C++ bitsSet function, though, time drops to around 7s.... which IS significantly faster. Tried something like res.length = 64; (in my D code), time dropped to around 11s (though slower than C++), but I'm not sure if the result is the same...

What further optimizations would you suggest for my D code, so that it's at least as fast as my C++ code?

Results with Compiler Optimization flags :

With clang++ bits.cpp -O3 -o cbits

time ./cbits

real    0m8.994s
user    0m8.986s
sys 0m0.006s

With dmd bits.d -O -release -inline -m64 -ofdbits

time ./bitsd

real    0m14.083s
user    0m14.034s
sys 0m0.014s

Which looks pretty amazing (or bizarre). Clang managed to go from 19 to 8 seconds, while D optimization did nothing???

EDIT : So, is there no hope that my D code will run as fast as it's C++ counterpart?

How about: using res.length = 64; and then in your loop replace

res ~= cast(int)log2(bitboard & ~(bitboard-1));

by:

res[n] = cast(int)log2(bitboard & ~(bitboard-1));

But to be honest this type of code will be completely dominated by allocations, which is not entirely interesting as a language benchmark. Try to reuse the same array instead.

And there is much faster ways of locating bits (see core.bitop).

tl;dr you're optimizing a slow program.


Single MySQL Query one-to-many efficiency I am fairly inexperienced with the ramifications of doing a single select where joining several tables and there are several one-to-many relationships while also running some built in MySQL functions using column values between tables.

The tables will be fairly large: 10,000 records in the accounts table, 70,0000 records in the locations table, 350,000 in the email_sequences table.

The select I am performing: (from Laravel's query builder)

$interval = $getSheduleInterval($current);

$sql = "SELECT seq.id FROM locations AS loc
        JOIN accounts as acc ON acc.id=loc.account_id
        JOIN email_sequence AS seq ON loc.id=seq.location_id
        WHERE acc.suspended = 0
        AND acc.deleted_at IS NULL
        AND loc.active = 1
        AND seq.immediate = 0
        AND TIME(CONVERT_TZ(seq.schedule_time, 'UTC', loc.timezone)) >= {$interval->start}
        AND TIME(CONVERT_TZ(seq.schedule_time, 'UTC', loc.timezone)) < {$interval->end}";

$sequents = DB::select($sql);

I have seen (read) where in some instances it is more efficient to perform several queries and process the result of each in between rather than a complex SELECT query where several one-to-many or many-to-one relationships are involved.

Are there going to be any major issues with this single select as I have it currently constructed?

Edit: As requested here is the $getScheduleInterval() code;

// @param  object [Takes a instance of Carbon::now()]
public function getScheduleInterval($carbonObj){
    $interval = (object)[];

    // round time down to quarter hour
    $carbonObj->minute = $carbonObj->minute - ($carbonObj->minute % 15);
    $carbonObj->second = 00;

    //set start / end
    $interval->start = "'{$carbonObj->toTimeString()}'";
    $interval->end =   "'{$carbonObj->addMinutes(15)->toTimeString()}'";

    return $interval;

}

I have seen (read) where in some instances it is more efficient to perform several queries and process the result of each in between rather than a complex SELECT query where several one-to-many or many-to-one relationships are involved.

Does that mean running a query to select data from each table and then subsequent queries to select the affiliated data? If so, repeated (redundant) queries (i.e. on one table where only a WHERE condition changes) can lead to poor performance. But, if the associated data can be shared across records (e.g. multiple accounts have the same location) then it might be better to have one query per table and then look up records in various result sets when needed.

If paging can be integrated, for example: populate the first \$n\$ results, where \$n\$ can be 20, 40, 60, etc. then the query with the joined tables might be optimal.

  Are there going to be any major issues with this single select as I have it currently constructed?

The query as it is above might likely take a while, given the high number of records in each table. But as mentioned above, paging would likely improve the situation greatly.


Recursive range iteration I decided to do Advent of Code this year in D, and the following is my solution to day 8.

Algorithm aside, I was specifically wondering if there was a better way to use ranges in a recursive manner here. I feel like I should be able to do write the functions in a more generic way (not just int[], avoiding the .array in main), and without destroying the original array in the process - avoiding the temporary variable p1.

My use of the header variable seems suspect to me as well - basically I want a popFront that returns a value, I think, but have been unable to find anything like that.

I'm also happy to hear general advice about usual best practices and efficiencies.

import std.algorithm;
import std.conv;
import std.file;
import std.range;
import std.stdio;
import std.string;

int count_metadata(ref int[] r)
{
    auto header = r.take(2);
    int num_child = header.front;
    int num_metadata = header.back;
    r = r.drop(2);
    int count;
    for (int i; i < num_child; i++) {
        count += r.count_metadata;
    }
    count += r.take(num_metadata).sum;
    r = r.drop(num_metadata);
    return count;
}

int node_value(ref int[] r)
{
    auto header = r.take(2);
    int num_child = header.front;
    int num_metadata = header.back;
    r = r.drop(2);
    int[] child_values;
    for (int i; i < num_child; i++) {
        child_values ~= node_value(r);
    }
    int count;
    if (num_child == 0) {
        count = r.take(num_metadata).sum;
    } else {
        // ignore out of range
        count = r.take(num_metadata).filter!(a => 0 < a && a <= child_values.length).map!(a => child_values[a - 1]).sum;
    }
    r = r.drop(num_metadata);
    return count;
}

void main(string[] args)
{
    // integers separated by spaces
    auto input = readText(args[1]).split.map!(a => to!int(a)).array;
    //auto input = [2, 3, 0, 3, 10, 11, 12, 1, 1, 0, 1, 99, 2, 1, 1, 2];
    auto p1 = input.dup;
    writeln("Metadata total: ", count_metadata(p1));
    writeln("Node value: ", node_value(input));
}

(args[1] is a file with integers separated by spaces, sample input is commented out below it.)

First, I'd prefer to parse the int[] into a Node[] or somesuch - I'd even argue the name of node_value is a code smell indicating that it should be a member called value of a type called Node. I believe this is part of what causes your consternation with the header variable.

Speaking of names, D naming conventions generally avoids underscores except for constants, so more idiomatic names would be nodeValue and countMetadata (which would perhaps more correctly be called sumMetadata).

One of the reasons you should parse to Node[] is it factors out the common parts between node_value and count_metadata, making their responsibilities clearer and the code easier to follow - remember, you're writing code for the next person who looks at it; the computer is happy with a string of bits that nobody can read.

Now, you mention genericity. For the most part, replacing (ref int[] r) with (R)(ref R r) if (isForwardRange!R) will deal with this. This templates the functions on the type of the range passed. Your code otherwise uses range functions (front, popFront, take, etc), so so no other change is necessary.

However, due to the structure of the file format, you will need to read all child nodes before getting at the metadata of a node. This limits the possible laziness of the solution. Since you are always iterating over every node, this doesn't matter a whole lot, but since that's one of the great benefits of ranges, it's worth pointing out.

As for consuming the range, there is no very simple, unobtrusive solution. You need to either create a temporary variable (like p1), have a duplicate function for the non-ref case, or use inner functions:

int count_metadata(R)(R r0)
if (isForwardRange!R)
{
    int impl(ref R r) {
        // Same code as before, but call impl instead of count_metadata
    }
    return impl(r);
}

You also mentioned efficiency. I don't think I can improve much on that. Both count_metadata and node_value are O(n), and my proposed solution doesn't change that.


Generating a selection box for the day of the month <select name="checkInDay" tabindex="3" onchange="calcDay();" class="ffe selectform" id="checkInDate">
    <?php for($i=1;$i<=31;$i++) {
        $value = $i < 10 ? "0".$i : $i;
     ?>
     <option value="<?= $value ?>" <?php if($i == 12) {?> selected="selected" <?php } ?> > <?= $i ?> </option>
     <?php } ?>

  </select>

I suggest to build a function which can be reused for the options:

function select_options($options, $selected = null) {
  $_options = '';
  foreach ($options as $value => $content) {
    $_options .= sprintf("<option value=\"%s\"%s>%s</option>\n", $value, $value == $selected ? ' selected="selected"' : '', $content);
  }

  return $_options;
}

And some PHP "magic" for the values generation but it is not really needed:

<?php $values = array_map(create_function('&$v', 'return sprintf("%02d", $v);'), range(1, 31)) ?>
<select name="checkInDay" tabindex="3" onchange="calcDay();" class="ffe selectform" id="checkInDate">
  <?php echo select_options(array_combine($values, range (1, 31)), 12) ?>
</select>

But more important than all: in a HTML template, use the PHP alternative notation.


Counting number of seen states I have some code which executes a simple task, counting the number of states reachable from a particular state, would just like some advice on how to improve it.

So far it stores all the states it has seen in seen_states. It's just counting them seems so clunky.

move([Head, Head1|Tail], [N|Tail]):-
    N is Head1 - Head,
    N >= 0.
move([Head|Tail], [Head|Tail1]):-
    move(Tail, Tail1).

:- dynamic counted_states/1.

count_state(S, V):-
    count_state(S,V, V).

count_state([], N,N).
count_state(S, Acc, Acc):-
    findall(X, move(S,X), Succ),
    count_sub_states(Succ, Acc, 0).

count_sub_states([],L,L).
count_sub_states([HeadList|List], Count, Acc):-
    count_node(HeadList,N),
    Acc1 is Acc+N,
    count_sub_states(List, Count, Acc1).

count_node(State, 0):-
    counted_states(State),!.

count_node(State, V):-
    not(counted_states(State)),
    assert(counted_states(State)),
    count_state(State, V1, V1),
    V is V1 +1.

As a general rule of thumb, you should avoid using the global database for such purposes.

For example, in your case, after the predicate completes, counted_states/1 is still asserted, and will influence later invocations of the predicate.

This certainly is not intended, no?

There is a straight-forward solution for this:

First, describe what it means that that a state is reachable from a particular state.
Then, use findall/3 to collect all such reachable states into a list of results.
On that list, simply use length/2 to obtain the number of reachable states.

This works without modifying the global database in any way.


Python get UTC offset for when only naive date time objects are available The function below is designed to return a UTC offset string, in the form used widely by timestamps, of the user's current time for when only naive date time objects are available (no time zone information).

I've tested it on two systems, which are in different time zones, and it works perfectly. However I'd like confirmation that it will work on all systems running Python versions >= 2.6. Can anyone see any potential problems?

Note: The function will only ever be used with the user's current time and so present or past dates and times are not an issue.

import time
from datetime import datetime

def get_utc_offset_str():
    """
    Returns a UTC offset string of the current time suitable for use in the
    most widely used timestamps (i.e. ISO 8601, RFC 3339). For example:
    10 hours ahead, 5 hours behind, and time is UTC: +10:00, -05:00, +00:00
    """

    # Calculate the UTC time difference in seconds.

    timestamp = time.time()
    time_now = datetime.fromtimestamp(timestamp)
    time_utc = datetime.utcfromtimestamp(timestamp)
    utc_offset_secs = (time_now - time_utc).total_seconds()

    # Flag variable to hold if the current time is behind UTC.
    is_behind_utc = False

    # If the current time is behind UTC convert the offset
    # seconds to a positive value and set the flag variable.
    if utc_offset_secs < 0:
        is_behind_utc = True
        utc_offset_secs *= -1

    # Build a UTC offset string suitable for use in a timestamp.

    if is_behind_utc:
        pos_neg_prefix = "-"
    else:
        pos_neg_prefix = "+"

    utc_offset = time.gmtime(utc_offset_secs)
    utc_offset_fmt = time.strftime("%H:%M", utc_offset)
    utc_offset_str = pos_neg_prefix + utc_offset_fmt

    return utc_offset_str

Take advantage of Python operator overloading

is_behind_utc = time_now < time_utc

If the current time is less the the UTC time then we are behind UTC time.

There is no need to use logic concerning seconds.

Longer names > comments

# Flag variable to hold if the current time is behind UTC.
is_behind_utc = False

Let's analyze the comment:

Flag variable : this can easily be deduced as it is a boolean
to hold if : each variable holds something
the current time : valuable information
is behind UTC : already present in the variable name

So I could write:

current_time_is_behind_utc = time_now < time_utc

The name incorporates the valuable information in the comment, avoids the unnecessary and the repetition of the same concept.

Use ternary when sensible

if is_behind_utc:
    pos_neg_prefix = "-"
else:
    pos_neg_prefix = "+"

I cringe because such verbosity obscures what is simple:

pos_neg_prefix = '-' if current_time_is_behind_utc else '+'

Simplify avoiding too many variables

utc_offset_fmt = time.strftime("%H:%M", utc_offset)
utc_offset_str = pos_neg_prefix + utc_offset_fmt

return utc_offset_str

Can simply be:

return pos_neg_prefix + time.strftime("%H:%M", utc_offset)

I think that each additional variable is an addition burden and point of failure in the code being another layer of indirection, and tend to avoid too many variables when sensible.


Value Object encapsulating different units of measure I have a situation where I need to use person's height in different units. Some parts of the system use Inches/Feet some Meters/Centimeters.

I was thinking of a Height value object which would encapsulate different measurement untis, and upon creation calculate and store all supported measurement units and provide accessors to get each.

I end up with this

public class Height : IEquatable<Height>
    {
        private Height()
        { }

        public double InMeters { get; private set; }
        public double InFeet { get; private set; }
        public double InInches { get; private set; }

        public static Height FromCMeters(double heightInCms) => new Height
        {
            InMeters = (double) heightInCms / 100,
            InFeet = heightInCms * 30.48,
            InInches = heightInCms * 2.54
        };

        public static Height FromFoots(double feet) => new Height
        {
            InFeet = feet,
            InMeters = (feet * 30.48) / 100,
            InInches = feet * 12
        };

        public static Height FromInches(double inches) => new Height
        {
            InInches = inches,
            InFeet = inches / 12,
            InMeters = (inches * 2.54) / 100
        };

        public bool Equals(Height other)
        {
            if (ReferenceEquals(null, other)) return false;
            if (ReferenceEquals(this, other)) return true;
            return InMeters.Equals(other.InMeters);
        }

        public override bool Equals(object obj)
        {
            if (ReferenceEquals(null, obj)) return false;
            if (ReferenceEquals(this, obj)) return true;
            if (obj.GetType() != this.GetType()) return false;
            return Equals((Height) obj);
        }

        public override int GetHashCode() => InMeters.GetHashCode();
    }

I intentionally omitted addition/subsctraction to keep code small.
The usage of Height value object:

var height = Height.FromCMeters(191);
// height.InMeters
// height.InInches
// height.InFeet

I have few doubts:

If I need to support new unit of measure I would have to change the value object which I don't think is a good idea
I tend to keep my value objects very small, this one seems to be doing too much for a value object (all those conversion logics)
I think it keeps track of many values instead of a one, and it feels that all this conversion logic belongs to somewhere else

This Height VO is mostly going to be used to represent height. All operations are going to be creating height from different units (m/cm/feet/inch) and pass it around. Whoever receives it is going to use it just to retrieve height value in the unit it needs.

This simple usage makes me think I don't really need more involved design, where I would have simply Height VO (with single metric) and let another component do the necessary conversion.

What do you think ?

Some quick remarks:

Don't pointlessly abbreviate: you gain nothing with "FromCMeters" or "heightInCms". Just use "Centimeters". I'd even argue that you shouldn't use the plural to express the height; I'd always say "I'm 189 centimeter". Same for "InMeters". (But I'm not a native English speaker.)
There are several magic numbers used repeatedly: 30.48, 2.54, 12. IMHO these should be const with a meaningful name.
I fully agree with Graipher's review: you should use one canonical unit and store its value in a private variable, and InMeter, InFeet and InInches should be "translations" of that unit (e.g. if you store it in cm, InMeter should divide the value by 100).


A Simple Media Generator with Directional Control My goal was to have a single page display bulk amounts of media without the need to load new pages. I also wanted it to be able to load fast, so the generator uses a JavaScript array to display one item at a time. The user can move through the array backwards and forwards with the use of two buttons.
Images, video and audio can be added as strings in the following ways:

'<video controls source src="FILE.mp4" type="video/mp4"></video>',
'',
'<audio controls source src="FILE.mp3" type="audio/mpeg"></audio>',

const array = [
    '',
    '
Text\
Hello I am text
',
    '<video controls source src="FILE.mp4" type="video/mp4"></video>',
    '',
    '<audio controls source src="FILE.mp3" type="audio/mpeg"></audio>',

];
//variables
const arrayLength = array.length;
var For = -1;

function backForth(ev) {
    if(ev.target.className == "buttonS") {
        const output = document.querySelector('output');
        let increment = (ev.target.textContent == "Back" ? -1 : 1);
        For = (For + increment + arrayLength) % arrayLength;
        output.innerHTML = array[For];
        document.getElementById('counter').innerHTML = For + '&nbsp;/&nbsp;' + (arrayLength - 1);
}}
document.addEventListener('click', backForth);
backForth({target:{className: "buttonS"}});

    <output></output>

        <button class="buttonS">Back</button>

        <button class="buttonS">Forth</button>

The overall approach looks fine. Some suggestions:
Delegated click logic The current backForth being used both as a click listener and on the initial load feels a bit clunky since you have to pass in {target:{className: "buttonS"}}. Consider making a separate function that you pass an index to increment (either 1 or -1), and call that inside backForth, and call it with (1) initially.
Don't move buttons User interfaces are slightly friendlier when buttons stay in the same place - when they don't move based on the content size. Consider either moving the buttons above the output, or giving the output a fixed (or high minimum) height.
For variable name is really weird - it's very close to for, which is a reserved word, and regardless doesn't really indicate what the variable contains very well. Consider naming it something like displayedMediaIndex.
Variable initializers - you use var, let, and const:

If you're going to write in ES6+ (which you are, and you should), always use const or let to declare variables. var has a few gotchas (such as an unintuitive function scope rather than block scope, and of automatically assigning itself to properties of the global object when on the top level.) See ESLint rule no-var.
When deciding between const and let, use const whenever possible, since it indicates that the variable won't be reassigned, which improves readability when a reader of the code can understand that at a glance.

Prefer strict equality over sloppy equality, since sloppy equality operators have some very strange type coercion rules that a reader of the code should not have to have memorized to be entirely confident of the logic your code is implementing.
This is how I'd do it:

const medias = [
    '',
    '
Text\
Hello I am text
',
    '<video controls source src="FILE.mp4" type="video/mp4"></video>',
    '',
    '<audio controls source src="FILE.mp3" type="audio/mpeg"></audio>',
];
const mediasLength = medias.length;
let displayedMediaIndex = -1;
const output = document.querySelector('output');
const paginator = document.querySelector('.paginator');
const changeMedia = (indexChange) => {
    displayedMediaIndex = (displayedMediaIndex + indexChange + mediasLength) % mediasLength;
    output.innerHTML = medias[displayedMediaIndex];
    paginator.textContent = displayedMediaIndex + ' / ' + (mediasLength - 1);
};
document.addEventListener('click', (event) => {
    if (event.target.className == 'buttonS') {
        changeMedia(event.target.textContent === "Back" ? -1 : 1);
    }
});
changeMedia(1);

    <button class="buttonS">Back</button>

    <button class="buttonS">Forth</button>

<output></output>


Division without using division, multiplication, or modulus operators The task

  Implement division of two positive integers without using the
  division, multiplication, or modulus operators. Return the quotient as
  an integer, ignoring the remainder.

My solution

const division = (dividend, divisor) => {
    let remainder = null;
    let quotient = 1;
    const sign = ((dividend > 0 && divisor < 0) ||
                 (dividend < 0 && divisor > 0)) ?
                 ~1 : 1;

    let tempdividend = Math.abs(dividend);
    let tempdivisor = Math.abs(divisor);

    if (tempdivisor === tempdividend) {
        remainder = 0;
        return sign;
    } else if (tempdividend < tempdivisor) {
        remainder = dividend < 0 ?
            sign < 0 ? ~tempdividend : tempdividend :
            tempdividend;
        return 0;
    }
    while (tempdivisor << 1 <= tempdividend) {
        tempdivisor = tempdivisor << 1;
        quotient = quotient << 1;
    }

    quotient = dividend < 0 ?
        (sign < 0 ? ~quotient : quotient) + division(~(tempdividend-tempdivisor), divisor) :
        (sign < 0 ? ~quotient : quotient) + division(tempdividend-tempdivisor, divisor);
    return quotient;
 }

Is bitwise operation mandatory? There's a much simpler way

const divide = (dividend, divisor) => {
     let quotient = 0, neg = false;

     if( (dividend < 0 && divisor > 0) || (dividend > 0 && divisor < 0) ){ neg = true; }

     dividend = Math.abs(dividend);
     divisor = Math.abs(divisor);

     if(dividend < divisor) {return 0;}
     else if(dividend > 0 && divisor != 0){
          while(dividend >= divisor){
               dividend -= divisor;
               ++quotient;
          }
     } else { // handle what you want to do for those cases..}

     return neg ? -quotient : quotient;
}

You get your quotient and remainder is ignored. Just do a check if dividend is negative or divisor is 0.


Code downloads a text file from a website, saves it to local disk, and then loads it into a list for further processing My program opens a website and downloads a text file.  The text file is a simple file with one word per line. I save the file to local disk and then create a list to hold each line of the text file for later processing.  I would like to know if I am doing these first steps in a way that would be considered idiomatic Python and have I made any big mistakes that will hamper my efforts to expand on it later.

This is similar to an exercise in Think Python by Allen Downey.  He suggests using a browser to download the text file but I wanted to do it with Python.

import requests

def get_webpage(uri):
    return requests.get(uri)

def save_webpagecontent(r, filename):
    """ This function saves the page retrieved by get_webpage. r is the
    response from the call to requests.get and
    filename is where we want to save the file to in the filesystem."""

    chunk_size = 8388608                # number of bytes to write to disk in each chunk
    with open(filename, 'wb') as fd:
        for chunk in r.iter_content(chunk_size):
            fd.write(chunk)

def make_wordlist(filename):
    wordlist = []
    with open(filename) as fd:
        wordlist = fd.readlines()
    return wordlist

def get_mylist(wordlist, num_lines=10):
    if len(wordlist) <= num_lines:
        return wordlist
    return wordlist[:num_lines]

def print_mylist(mylist):
    for word in mylist:
        print(word.strip())
    return None

"""List of words collected and contributed to the public domain by
Grady Ward as part of the Moby lexicon project. See https://en.wikipedia.org/wiki/Moby_Project
"""
uri = 'https://ia802308.us.archive.org/7/items/mobywordlists03201gut/CROSSWD.TXT'
filename = 'wordlist.txt'

r = get_webpage(uri)
save_webpagecontent(r, filename)
wordlist = make_wordlist(filename)
mylist = get_mylist(wordlist)
print_mylist(mylist)

My program works as I expect it to.  I have basically found how to do each individual piece by reading this forum and but I would like to know if I'm putting all the pieces together correctly.  By correctly I mean something that not only functions as expected but also will be easy to build larger programs and modules from.

I hope it isn't wrong of me to post this much code.  I wasn't sure how I could trim it down and still show what I am doing.  Please let me know if I need to change the format of my question.

Your code is nice and concise however there are some changes you can make:

You can just return f.readlines() in make_wordlist.
If you've done this to show that the result is a list then it'd be better to use the typing module.

from typing import List

def make_wordlist(filename: str) -> List[str]:
    ...

get_mylist can be replaced with wordlist[:numlines]. This is because if len(wordlist) is smaller or equal to numlines, then it will return the entire thing anyway.
Performance wise it's best to use print('\n'.join(list)) rather than for item in list: print(item).
I would prefer to be able to change chunk_size in save_webpagecontent and so you can make it a default argument.
IIRC multi-line docstrings shouldn't start on the same line as the """, nor should they end on the same line either.

import requests
from typing import List

Response = requests.Response

def get_webpage(uri) -> Response:
    return requests.get(uri)

def save_webpagecontent(r: Response, filename: str,
                        chunk_size: int=8388608) -> None:
    """
    This function saves the page retrieved by get_webpage. r is the
    response from the call to requests.get and
    filename is where we want to save the file to in the filesystem.
    """
    with open(filename, 'wb') as fd:
        for chunk in r.iter_content(chunk_size):
            fd.write(chunk)

def read_wordlist(filename: str) -> List[str]:
    with open(filename) as fd:
        return fd.readlines()

def print_mylist(word_list: List[str]) -> None:
    print('\n'.join(word.strip() for word in word_list))

"""
List of words collected and contributed to the public domain by
Grady Ward as part of the Moby lexicon project. See https://en.wikipedia.org/wiki/Moby_Project
"""
uri = 'https://ia802308.us.archive.org/7/items/mobywordlists03201gut/CROSSWD.TXT'
filename = 'wordlist.txt'

r = get_webpage(uri)
save_webpagecontent(r, filename)
print_mylist(read_wordlist(filename)[:10])


Lookup an array against another with performance I got an initial array, I am checking against another array to find how many objects have at least one instance of the Domain in data.
I wonder if there is a more performant way to achieve the same goal.

const data = [
  {
    Domain: 'google.com',
    '# Reocurring Domains': 0
  },
  {
    Domain: 'apple.com',
    '# Reocurring Domains': 0
  },
  {
    Domain: 'facebook.com',
    '# Reocurring Domains': 0
  }
]

const domains = [
  {
    'google.com': true,
    'microsoft.com': true,
    'google.com': true
  },
  {
    'apple.com': true,
    'microsoft.com': true,
    'twitter.com': true
  },
  {
    'facebook.com': true,
    'apple.com': true,
    'facebook.com': true
  }
]

for (const obj of data) {
  let count = 1
  for (const entry of domains) {
    if (entry[obj.Domain]) {
      obj['# Reocurring Domains'] = count++
    }
  }
}

console.log(data)

In there any way to this with a more performant approach?

You could create a look up table (a simple object) where you can pre calculate the amount of times a domain is in the repeated in the domains array, this reduces the algorithmic complexity of your current solution which is n * m (dataElements  * domainElements)
Whereas my suggestion has a lower complexity of (n + m)

const data = [
    {
        Domain: 'google.com',
        '# Reocurring Domains': 0
    },
    {
        Domain: 'apple.com',
        '# Reocurring Domains': 0
    },
    {
        Domain: 'facebook.com',
        '# Reocurring Domains': 0
    }
]

const domains = [
    {
        'google.com': true,
        'microsoft.com': true,
        'google.com': true
    },
    {
        'apple.com': true,
        'microsoft.com': true,
        'twitter.com': true
    },
    {
        'facebook.com': true,
        'apple.com': true,
        'facebook.com': true
    }
]

const domainsObj = {}

domains.forEach(domain => {
    Object.keys(domain).forEach(key => key in domainsObj ? domainsObj[key] += 1: domainsObj[key] = 1)
})

data.forEach(element => element['# Reocurring Domains'] = domainsObj[element.Domain]);

console.log(data);

This approach increases memory usage, but improves calculation speed.


Polynomial implementation in Golang I am writing a numerical analysis library in golang for which I need to implement a polynomial struct. Here is the source code:

package numerical

import (
    "strconv"
)

type Poly struct {
    Coeffs []float64
}

func (p *Poly) Str() string {
    d := p.Deg()

    if d == -1 {
        return "0"
    }

    s := ""
    if p.Coeffs[d] == -1 {
        if d == 0 {
            s += "-1"
        } else {
            s += "-"
        }
    } else if p.Coeffs[d] != 1 || d == 0 {
        s += strconv.FormatFloat(p.Coeffs[d], 'g', -1, 64)
    }
    if d == 1 {
        s += "x"
    } else if d > 1 {
        s += "x^" + strconv.Itoa(d)
    }
    for i := d-1; i >= 0; i-- {
        if p.Coeffs[i] > 0 {
            s +=  " + "
            if p.Coeffs[i] != 1 || i == 0 {
                s += strconv.FormatFloat(p.Coeffs[i], 'g', -1, 64)
            }
        } else if p.Coeffs[i] < 0 {
            s += " - "
            if p.Coeffs[i] != -1 || i == 0 {
                s += strconv.FormatFloat(-p.Coeffs[i], 'g', -1, 64)
            }
        }
        if p.Coeffs[i] != 0 {
            if i == 1 {
                s += "x"
            } else if i > 1 {
                s += "x^" + strconv.Itoa(i)
            }
        }
    }

    return s
}

func (p *Poly) Deg() int {
    i := len(p.Coeffs) - 1
    for ; i >= 0 && p.Coeffs[i] == 0; i-=1 {
    }
    return i
}

func (p *Poly) Eval(x float64) float64 {
    val := p.Coeffs[0]
    pow := 1.0
    for i:=1; i <= p.Deg(); i+=1 {
        pow *= x
        val += p.Coeffs[i]*pow
    }
    return val
}

func PolyAdd(p, q Poly) Poly {
    var big, small, r Poly
    if p.Deg() > q.Deg() {
        big = p
        small = q
    } else {
        big = q
        small = p
    }
    r = big
    for i := 0; i <= small.Deg(); i+=1 {
        r.Coeffs[i] += small.Coeffs[i]
    }
    return r
}

func PolySub(p, q Poly) Poly {
    degp := p.Deg()
    degq := q.Deg()
    big := degp
    if degp < degq {
        big = degq
    }
    r := Poly{make([]float64, big+1)}
    for i := 0; i <= big; i++ {
        if i <= degp {
            r.Coeffs[i] = p.Coeffs[i]
        }
        if i <= degq {
            r.Coeffs[i] -= q.Coeffs[i]
        }
    }
    return r
}

func PolyMul(p, q Poly) Poly {
    m, n := p.Deg(), q.Deg()
    r := Poly{make([]float64, m+n+1)}
    for i := 0; i <= m; i++ {
        for j := 0; j <= n; j++ {
            r.Coeffs[i+j] += p.Coeffs[i]*q.Coeffs[j]
        }
    }
    return r
}

func PolyDiv(a, b Poly) (Poly, Poly) {
    var q Poly
    for b.Deg() <= a.Deg() {
        dega := a.Deg()
        degb := b.Deg()
        qc := a.Coeffs[dega]/b.Coeffs[degb]
        qp := dega - degb
        qi := Poly{make([]float64, qp+1)}
        qi.Coeffs[qp] = qc
        a = PolySub(a, PolyMul(qi, b))
        q = PolyAdd(q, qi)
    }
    return q, a
}

A few things that I observed about the code are:

The Poly.Deg is a method. I should probably make a non-method field (what are they called?) for that purpose, but then I will have to create a Poly.UpdateDeg which will set the degree for a new polynomial, and then I will have to call this UpdateDeg in all the other functions like PolyAdd, PolyMul etc. Is their a better way to achieve this.
The Poly.Str method, which basically pretty-prints the polynomial looks very complicated. The purpose of this method is to return a string containing the polynomial in human readable form. For example, the polynomial {-1, 2, -1} will give -x^2 + 2x - 1 (pay special attention to the leading coefficients and spaces around + and -). Can this method be simplified?

Any other suggestion/improvements are welcome, in particular, I would like to know if the code is idiomatic Go. I am new to Go, and would like to learn the Go-way of doing things.

EDIT: Here is the code on Playground (slightly modified to run on Playground).

Your code is missing automatic tests.

One test case you definitely need is to ensure that PolyAdd and the related functions do not modify their arguments. As far as I can see by reading the code, the current version of PolyAdd modifies the bigger of its arguments, which is unexpected.

The reason that the argument is modified is that slices are essentially pointers, and when passing them as arguments, their content is not copied.

You should define a constructor function NewPoly(deg int) to hide all the implementation details. Using this constructor will also solve the aliasing problem mentioned above.

Regarding the long code for Str, you can merge some of the duplicate code:

    c := p.Coeffs[i]
    if c == 0 {
        continue;
    }

    if c > 0 {
        s += " + "
    } else {
        s += " - "
    }

    if (c != 1 && c != -1) || i == 0 {
        s += strconv.FormatFloat(c, 'g', -1, 64)
    }
    if i > 1 {
        s += "x^" + strconv.Itoa(i)
    } else if i == 1 {
        s += "x"
    }

By extracting the expression p.Coeffs[i] into a variable, the code gets a bit cleaner.

If you need to speed up Str, use a strings.Builder instead of the += operator.


Testing for divisibility of numbers - follow-up (Here is the follow up question)

I had another go at trying to speed up my program described in my previous question.

JS1's answer was particulary helpful and now my code is only about 20% slower than the fastest program in the programming challange (it runs in 0,13 s and my code runs in 0,16 s). I ran the code with different tools to check how often each line was run, etc.

#include <stdio.h>
#define MAX 1000000

int main() {
  int N,n[100];
  int p[10] = {0};
  int mask[MAX] = {0};
  register int i, j, k;

  scanf("%d", &N);
  for(i=0;i<N;i++) scanf("%d", &n[i]);
  i = MAX+1;
  while(--i) {
    int maskBits = 0;
    for(j=0;j<N;j++) {if(i%n[j]==0)maskBits |= (1 << j);}
    if (maskBits == 0)
        continue;
    k=i;
    while (k>0) {p[k%10]++; k/=10;}

    for(j=1;j<10;j++) if(p[j]>0) {k=j; p[j]--; break;}
    for(j=0;j<10;j++) while (p[j]>0) {k=10*k+j; p[j]--;}
    mask[k] |= maskBits;
  }
  for(i=1;i<MAX;i++) if(mask[i]==(1<<N)-1) break;
  fprintf(stdout, "%d\n", i);
}

Again I ran the code through gcov and here is some extensive information about my program.

        -:    0:Source:talfamiljer.c
        -:    0:Graph:talfamiljer.gcno
        -:    0:Data:talfamiljer.gcda
        -:    0:Runs:1
        -:    0:Programs:1
        -:    1:#include <stdio.h>
        -:    2:#define MAX 1000000
        -:    3:
        1:    4:int main() {
        -:    5:  int N,n[100];
        1:    6:  int p[10] = {0};
        1:    7:  int mask[MAX] = {0};
        -:    8:  register int i, j, k;
        -:    9:
        1:   10:  scanf("%d", &N);
        1:   11:  for(i=0;i<N;i++) scanf("%d", &n[i]);
        -:   12:  i = MAX+1;
  1000001:   13:  while(--i) {
        -:   14:    int maskBits = 0;
  7000000:   15:    for(j=0;j<N;j++) {if(i%n[j]==0)maskBits |= (1 << j);}
  1000000:   16:    if (maskBits == 0)
   941611:   17:        continue;
        -:   18:    k=i;
   343849:   19:    while (k>0) {p[k%10]++; k/=10;}
        -:   20:
   116542:   21:    for(j=1;j<10;j++) if(p[j]>0) {k=j; p[j]--; break;}
   285460:   22:    for(j=0;j<10;j++) while (p[j]>0) {k=10*k+j; p[j]--;}
    58389:   23:    mask[k] |= maskBits;
        -:   24:  }
   102246:   25:  for(i=1;i<MAX;i++) if(mask[i]==(1<<N)-1) break;
        1:   26:  fprintf(stdout, "%d\n", i);
        -:   27:}

Something that caught my eye is that the first while loop runs 1000001. However, it should only have to run until it finds the smallest number whose permutations or itself is divisble my all the numbers in the input file.

7
164 278 293 382 483 598 23

I'd be very happy if you can help me beat the fastest program.

Precomputed table

I was able to get a much faster result by doing the following:

I precomputed a table which listed for each number, all the permutations of that number.  Only the lowest valid number contained its permutations.  For example, the number 102 contained the permutations: 102 120 201 210.  The entries for 120, 201, and 210 were empty.
I iterated through each number from 1 to MAX.  For each number, I checked whether each divisor divided into at least one of the permutations of the number.  If all divisors divided at least once, then the program was done and the lowest number was found.

When I say I precomputed a table, I mean that I wrote a separate program to dump a table of 1000000 ints into a C file.  I then modified that C file and turned the table into an array of 1000000 ints (a large file > 10MB, but only using about 4 MB at run time), and compiled that huge array into my program.  So I had something that looked like this:

int bigArray[] = {
    -1, -2, -3, -4, ..., -12, 21, -13, 31, ...,
    -102, 120, 201, 210, -103, ..., -999999, -1000000
};

I used negative numbers to denote the start of a new permutation and positive numbers to denote permutations of the entry.  Thus, the -12 and 21 above mean that 12 is the start of a new permutation and 21 is one of the permutations for it.

I have the code to generate the table and the code to use the table, but rather than post them here, I will leave it to the OP to use this idea and create their own code.

The precomputed table version seemed to be about 2x as fast as the previous version, but since the times were so small (0.03 vs 0.06 seconds), it's hard to say whether that measurement was really accurate.


python function compression I have written this big function to do some formatting in my python code. Would you be able to suggest anyways to make this smaller ?

def disfun(String1,String2,String3):
        if String3 == "A" or String3 == "B":
            if String3 == "A":
                pass
            elif String3 == "B":
                print "#"*54

            print "##"," "*48,"##"
            print "##",'{0:^48}'.format(String2),"##"
            print "##",'{0:^48}'.format(String1),"##"
            print "##"," "*48,"##"
            print "#"*54

        elif String3 == "C":
            print "-"*40
            print "--",'{0:^34}'.format(String2),"--"
            print "-"*40

        elif String3 == 'D':
            String2 = ' * '.join(String2)
            print "#"*54
            print "##",'{0:^48}'.format(String2),"##"
            print "##",'{0:^48}'.format(String1),"##"
            print "#"*54

        elif String3 == 'E':
            print "*"*54
            print "**",'{0:^48}'.format(String2),"**"
            print "**",'{0:^48}'.format(String1),"**"
            print "*"*54

Ok, the first thing is that the names are not at all descriptive. disfun? What does that mean? Maybe something like generateReport? What are the purposes of String1 and String2? Why does String2 get printed before String1? Why does String1 not get printed at all when String3 is "C"? Also, python variables tend to begin with a lowercase letter, not an uppercase letter.

As to shortening the function, there's a rule of thumb called DRY: Don't Repeat Yourself. If you see the same code written several times, extract it into a function.

Here's a first shot at improving your code:

def __printBannerEdge(char, width):
    assert len(char) == 1
    print char * width

def __printBannerContent(char, width, content):
    assert len(char) == 1
    print char * 2, ('{0:^' + str(width - 6) + '}').format(content), char * 2

def __printBanner(char, width, lines):
    __printBannerEdge(char, width)
    for line in lines:
        __printBannerContent(char, width, line)
    __printBannerEdge(char, width)

def generateReport(content, title, selection):
    if selection == "A" or selection == "B":
        if selection == "B":
            __printBannerEdge('#', 54)
        __printBannerContent('#', 54, '')
        __printBannerContent('#', 54, title)
        __printBannerContent('#', 54, content)
        __printBannerContent('#', 54, '')
        __printBannerEdge('#', 54)

    elif selection == "C":
        __printBanner('-', 40, [title])

    elif selection == 'D':
        __printBanner('#', 54, [' * '.join(title), content])

    elif selection == 'E':
        __printBanner('#', 54, [title, content])

Some commentary:

The names I use (content, title, selection) are guess from context. There may be better names possible.
I'm a little confused as to why the output is so different for each report type. I'm guessing that selection is set by user input from a menu of some sort? You should separate the function that parses user input from this function, and have 5 separate functions (eg generatePartList).
One of the oddest thing about the function as written is that sometimes title is a single string, and sometimes it seems to be a list of strings (or maybe the join statement is meant to "bold" the title name?).
Better than printing in these functions is to return a string, and let the caller print the string (that makes it easier to write your output to a file instead of standard output, for instance, if you decide later that's something you want to do).


Find two missing elements from a sorted array in given range Looking for reviewers to suggest optimization and improvements, and to evaluate coding practices.

final class Variables {
    private final int x;
    private final int y;

    Variables(int x2, int y2) {
        this.x = x2;
        this.y = y2;
    }

    public int getX() {
        return x;
    }

    public int getY() {
        return y;
    }

    @Override public String toString()  {
        return "x = " + x + " y = " + y;
    }
}

public final class FindMissing {

    private FindMissing() {};

    /**
 * Takes an input a sorted array with two variables that repeat in the
 * range. Remaining variable are consecutive. and returns two variables that repeats
 *
 * eg: for a range of 1,2,3,4, a valid input is [1, 4], [1, 3] etc.
 *
 * @param a : the sorted array.
 */
/*
 * Why is start needed ?
 * If start is not given then if a1 is empty how do we know if missing numbers were
 * [1, 2] or [4, 5] ??.
 */
public static Variables sortedConsecutiveTwoMissing (int[] a, int startOfRange) {
    if (a == null) throw new NullPointerException("a1 cannot be null. ");

    int low = startOfRange;
    // calculating the highest value of the range. it needs to be 2 more than length since all elememts are consecutive.
    int high = startOfRange + (a.length - 1) + 2;

    int i = 0;
    boolean foundX = false;
    int x = 0;

    while (i < a.length) {
        if (a[i] != low) {
            if (foundX) {
                return new Variables(x, low);
            } else {
                x = low;
                foundX = true;
            }
        } else {
            i++;
        }
        low++;
    }

    return new Variables(x, high);
 }

public static void main(String[] args) {
        // unsortedConsecutiveTwoMissing
        int[] a6 = {1, 4};
        System.out.println("Expected 2 and 3, Actual " + sortedConsecutiveTwoMissing(a6, 1));
        // unsortedConsecutiveTwoMissing
        int[] a7 = {-4, -1};
        System.out.println("Expected -3 and -2, Actual " + sortedConsecutiveTwoMissing(a7, -4));
        int[] a8 = {-4, -3, -2, -1};
        System.out.println("Expected 0 and 1, Actual " + sortedConsecutiveTwoMissing(a8, -4));
        int[] a9 = {5};
        System.out.println("Expected 4 and 6, Actual " + sortedConsecutiveTwoMissing(a9, 4));
        }
     }

You say the problem is to find missing values from a 'Range', but your method only takes a start value, not an end value.... is this intentional? The method comment says:

  Takes an input a sorted array with two variables that repeat in the range. and returns two variables that repeats

The method does nothing of the sort though... it takes one variable, and there's no indication the values repeat, and it returns missing values, not repeats.

Additionally, the method name is sortedConsecutiveTwoMissing but there's nothing in the implementation that suggests that the values have to be consecutive at all...

Also you suggest the array is sorted, but not whether it is unique... for example, your code may break on (missing is 3 and 5):

0 1 2 2 4 4 6 6 7 8 9

General / Code Style

Variables like a1 make me want to look for a2, a3, etc. Unless it's obvious, don't use numbered variables.
int high = start + (a1.length - 1) + 2; should have a comment .... the + 2 is not obvious, and magic numbers should either be very obvious, or documented. That line still makes no sense to me at all.... and high is not used for anything reasonable anyway.
calculatedX is not a calculated anything

At this point I don't believe the code does what you suggest it should, so I don't believe the code works... downvote time.

Edit the code, make it work....


argmax of List of Double Here is my implementation of arg max in Scala. q_table_test.txt contains:
["---------,'0','0','0','0','0','0','0','0','0', "X--------,'0.', '0.1', '0.1', '0.1', '0.1', '0.1', '0.1', '0.1', '0.1'"]

For example the String ---------,'0','0','0','0','0','0','0','0','0' maps to key: "---------" , with values: [0,0,0,0,0,0,0,0,0]
After filtering values which are not valid I return the arg max of values from the list of positions.
Coming from a java background, what is a more functional programming principled method of implementing arg max?
Should the list filtering be contained in a separate function?
The QTable apply() method parses reads a file and returns Map[String , List[Double]] . It is evaluated lazily as it should just be evaluated once.
I think the use of apply in lazy val is not correct ?
Is this a 'good' method of finding the arg max value ?:
val qValues = bindings.get(state).getOrElse(List.fill(8)(0.0)).zipWithIndex
val availableQValueBoardPositions = qValues.filter(f => remainingPositions.contains(f._2))

Complete code:
import java.io.InputStream
import play.api.Environment

case class QTable(bindings: Map[String, List[Double]]) {

  def getArgMaxValue(state: String, remainingPositions: List[Int]) = {
    val qValues = bindings.get(state).getOrElse(List.fill(8)(0.0)).zipWithIndex
    val availableQValueBoardPositions = qValues.filter(f => remainingPositions.contains(f._2))
    availableQValueBoardPositions.maxBy(x => x._1)._2
  }

}

object QTable {

  private def getQTableFromFile(filename: String) = {
    lazy val env = Environment.simple()
    lazy val is: InputStream = Option(env.classLoader.getResourceAsStream(filename)).get
    scala.io.Source.fromInputStream(is).mkString
  }

  private def cleanData(data: String): String = {
    data.replace("\"", "").replace("\'", "").replace("[", "").replace("]", "")
  }

  def apply(filename: String) = {

    val number_attributes_per_instance = 10;

    val qtable = getQTableFromFile(filename)

    lazy val cleanedQtable: String = cleanData(qtable)
    lazy val dataInstances: List[List[String]] = cleanedQtable.split(",").toList.grouped(number_attributes_per_instance).toList
    lazy val stateIdValues: List[String] = dataInstances.map(m => m.head.trim)
    lazy val stateAttributeValues: List[List[Double]] = dataInstances.map(m => m.tail.map(x => x.toDouble))
    lazy val policy: QTable = new QTable(stateIdValues.zip(stateAttributeValues).toMap)

    policy
  }

}

object ArgMain extends App {
  print(QTable("q_table_test.txt").getArgMaxValue("---------", List(1, 2, 3)))
}

So you want the index of the maximum value in your collection. That's a lot of code for such a simple task.
Let's start at the top.
q_table_test.txt - The sample data supplied is rather ridiculous. The elements that will become "attribute values" are all the same. How do you know the code is correct if there is no maximum? Does it matter which index is returned?
QTable.apply(filename) - The number_attributes_per_instance should be a configuration parameter, or derived directly from the input data. What if you needed to support more than one number_attributes_per_instance?
You don't appear to know what a lazy val is and what it's for. Its usage here makes no sense.
There are no safety checks. The file text is loaded as if formatting errors can't happen.
getQTableFromFile() - Why is getResourceAsStream() wrapped in an Option and then unwrapped with a .get? That's like loaning your brother a dollar so that he can repay the dollar he borrowed from you yesterday. A lot of busy work that accomplishes nothing.
cleanData() - Very inefficient.
getArgMaxValue() - If the state parameter is faulty you go through the trouble of building a bogus qValues list and then the elaborate and convoluted steps of finding the index of its max value, of which there is none. If you're going to lie to the user why not just return remainingPositions.head and be done with it?

As a demonstration, here's a smaller yet safer implementation of the same basic outline.
import scala.util.{Try, Using}  //Scala 2.13.x

class QTable(bindings: Map[String, Array[Double]]) {
  def getMaxValueIdx(state              : String
                    ,remainingPositions : List[Int]
                    ) :Option[Int] =
    bindings.get(state)
            .flatMap(arr => remainingPositions.maxByOption(arr.lift))
}

object QTable {
  private def loadFromFile(filename :String) :Try[String] =
    Using(scala.io.Source.fromFile(filename)) {
      _.getLines().map(_.takeWhile(_ != '#').trim).mkString
    }

  private val recFormat =
    """([^,"\[\]]+)\s*,((\s*('\d*\.?\d*')\s*,?)+)""".r.unanchored

  def apply(filename :String) :QTable =
    loadFromFile(filename).flatMap{ data => Try {
      recFormat.findAllMatchIn(data)
        .map(m => m.group(1) -> m.group(2)
                                 .split("[,\\s]+")
                                 .collect{case s"'$n'" => n.toDouble}
            ).toMap
    }}.fold(err => {println(err); new QTable(Map.empty)}
           ,new QTable(_))
}

My q_table_test.txt file:
[
"1st 9 Tbl,'0','1','2','3','4','5','6','7','8', #table of 9 elements
"Short Tbl,   '7' , '1.1','.22', '3'  ,'12.0' , #mixed spacing
"Split Tbl, '17', '1.25', '.127', '300.003'     #1st part
          , '12.0', '4321', '.1234'           , #2nd part
"2nd 9 Tbl,'0.','0.9','0.8','0.7','0.6','0.5','0.4','0.3','0.2'"
]

testing:
val qx = QTable("bogus.txt")    // (No such file or directory)
qx.getMaxValueIdx("1st 9 Tbl", List(7, 2, 3))  // None

val qt = QTable("./q_table_test.txt")
qt.getMaxValueIdx("1st 9 Tbl", List(7, 2, 3))  // Some(7)
qt.getMaxValueIdx("1st 9 Tbl", List())         // None
qt.getMaxValueIdx("Short Tbl", List(2,3,0))    // Some(0)
qt.getMaxValueIdx("Short Tbl", List(2,3,990))  // Some(3)
qt.getMaxValueIdx("NoSuchTbl", List(5,0,8))    // None
qt.getMaxValueIdx("Split Tbl", List(5,0,8))    // Some(5)
qt.getMaxValueIdx("2nd 9 Tbl", List(7, 2, 3))  // Some(2)


Acquiring employee information I am curious to know if there is a faster, better, and more efficient way to accomplish this program that acquires employee information:

    <?php
session_start();
//require 'functions.php';
//require 'DB.php';
$employeeID = $_SESSION['employeeID'];
$clockIn = $_GET['clockIn'];
$clockOut = $_GET['clockOut'];
$timeToday = date("g:i a");
$dateToday = date("m/d/y");
$jobDescription = $_GET['jobDesc'];
$equipType = $_GET['equipTypeRan'];
$unitNumber = $_GET['unitNumber'];
$unitHours = $_GET['unitHours'];
if (isset($clockIn)) {

    echo "You clocked in at: " . $timeToday . " on " . $dateToday;

    try {
    $conn = new PDO('mysql:host=localhost;dbname=timecard', 'username', 'password');
    $conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
    $stmt = $conn->prepare('INSERT INTO timeRecords (employeeID, date, timeIn) VALUES (:employeeID, :dateToday, :timeIn)');

    $stmt->execute(array(':employeeID' => $_SESSION['employeeID'], ':dateToday' => $dateToday, ':timeIn' => $timeToday));

    } catch(PDOException $e){
        echo'ERROR: ' . $e->getMessage();
    }
    $conn = null;

} else if (isset($clockOut)) {

        try {
    $conn = new PDO('mysql:host=localhost;dbname=timecard', 'username', 'password');
    $conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
    $stmt = $conn->prepare('UPDATE `timeRecords` SET `timeOut`= :timeOut WHERE `date`= :dateToday AND `employeeID`= :employeeID');

    $stmt->execute(array(':employeeID' => $_SESSION['employeeID'],':dateToday' => $dateToday, ':timeOut' => $timeToday));

    $stmt = $conn->prepare('SELECT `timeIn` FROM `timeRecords` WHERE `date`= :dateToday AND `employeeID` = :employeeID');
    $stmt->execute(array(':employeeID' => $_SESSION['employeeID'], ':dateToday' => $dateToday));
    $stmt->setFetchMode(PDO::FETCH_BOTH);
    $results = $stmt->fetch();
    $timeInDB = $results[0];

    } catch(PDOException $e){
        echo'ERROR: ' . $e->getMessage();
    }

    $time_one = new DateTime($timeInDB);
    $time_two = new DateTime($timeToday);
    $difference = $time_one->diff($time_two);

    echo "You clocked in at: " . $timeInDB . "
";
    echo "You clocked out at: " . $timeToday . "
";

    echo $difference->format('Total working time %h hours %i minutes');

}else {

try{
    $conn = new PDO('mysql:host=localhost;dbname=timecard', 'username', 'password');
    $conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
    $stmt = $conn->prepare("SELECT COUNT(*) FROM timeRecords WHERE `timeOut` IS NULL AND `employeeID`= :employeeID AND `date`= :dateToday");

    $stmt->execute(array(':employeeID' => $employeeID, ':dateToday' => $dateToday));
    } catch(PDOException $e){
        echo'ERROR: ' . $e->getMessage();
    }

if($stmt->fetchColumn() > 0){

    try{
    $conn = new PDO('mysql:host=localhost;dbname=timecard', 'username', 'password');
    $conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
    $stmt = $conn->prepare('UPDATE `timeRecords` SET `jobDescription`= :jobDescription, `equipType`= :equipType, `unitNumber`= :unitNumber, `unitHours`= :unitHours,
                            `timeOut`= :timeOut WHERE `employeeID`= :employeeID AND `date`= :dateToday AND `timeOut` IS NULL');

    $stmt->execute(array(':employeeID' => $employeeID, ':timeOut' => $timeToday, ':dateToday' => $dateToday, ':jobDescription' => $jobDescription,
                         ':equipType' => $equipType, ':unitNumber' => $unitNumber, ':unitHours' => $unitHours));

    $stmt = $conn->prepare('INSERT INTO timeRecords (employeeID, date, timeIn) VALUES (:employeeID, :dateToday, :timeIn)');
    $stmt ->execute(array(':employeeID' => $employeeID, ':dateToday'=> $dateToday, ':timeIn' => $timeToday));
    } catch(PDOException $e){
        echo'ERROR: ' . $e->getMessage();
    }

    echo "There was an update";
} else {
    echo "There was no Match";
}

    require 'summary.php';

}

?>

Defining the same connection multiple times can be avoided, and also, putting everything in try/catch serves no purpose if the only thing you do in the catch is displaying the message which would be displayed anyway with the error reporting mode you set on the $conn

I also find that naming PDOStatements can help a bit for readability, instead of always using $stmt. Here it all happens in few lines, but sometimes, using a prepared statement far away in the code can be easier if they have significant names.

Doing this, I noticed your clockIn PDOStatement is done twice. Since the if prevents it from happening twice, it's OK, but perhaps you could find some use in preparing your PDOStatements outside of loops, then executing them in the loops.

<?php

session_start();
//require 'functions.php';
//require 'DB.php';
$employeeID = $_SESSION['employeeID'];
$clockIn = $_GET['clockIn'];
$clockOut = $_GET['clockOut'];
$timeToday = date("g:i a");
$dateToday = date("m/d/y");
$jobDescription = $_GET['jobDesc'];
$equipType = $_GET['equipTypeRan'];
$unitNumber = $_GET['unitNumber'];
$unitHours = $_GET['unitHours'];

$conn = new PDO('mysql:host=localhost;dbname=timecard', 'username', 'password');
$conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

$putClockIn = $conn->prepare('INSERT INTO timeRecords (employeeID, date, timeIn) VALUES (:employeeID, :dateToday, :timeIn)');

if (isset($clockIn)) {
    echo "You clocked in at: " . $timeToday . " on " . $dateToday;
    $putClockIn->execute(array(
        ':employeeID' => $_SESSION['employeeID'],
        ':dateToday' => $dateToday,
        ':timeIn' => $timeToday
    ));
    $conn = null;
} else {
    if (isset($clockOut)) {
        $putClockOut = $conn->prepare('UPDATE `timeRecords` SET `timeOut`= :timeOut WHERE `date`= :dateToday AND `employeeID`= :employeeID');
        $putClockOut->execute(array(
            ':employeeID' => $_SESSION['employeeID'],
            ':dateToday' => $dateToday,
            ':timeOut' => $timeToday
        ));
        $getTimeIn = $conn->prepare('SELECT `timeIn` FROM `timeRecords` WHERE `date`= :dateToday AND `employeeID` = :employeeID');
        $getTimeIn->execute(array(':employeeID' => $_SESSION['employeeID'], ':dateToday' => $dateToday));
        //$getTimeIn->setFetchMode(PDO::FETCH_BOTH);
        $results = $getTimeIn->fetch(PDO::FETCH_BOTH); // no need to set a mode for stmt if you ain't using it anywhere else, pass it directly in fetch()
        $timeInDB = $results[0];
        $time_one = new DateTime($timeInDB);
        $time_two = new DateTime($timeToday);
        $difference = $time_one->diff($time_two);
        echo "You clocked in at: " . $timeInDB . "
";
        echo "You clocked out at: " . $timeToday . "
";
        echo $difference->format('Total working time %h hours %i minutes');
    } else {
        $getCurrentSessions = $conn->prepare("SELECT COUNT(*) FROM timeRecords WHERE `timeOut` IS NULL AND `employeeID`= :employeeID AND `date`= :dateToday");
        $getCurrentSessions->execute(array(':employeeID' => $employeeID, ':dateToday' => $dateToday));
        if ($getCurrentSessions->fetchColumn() > 0) {
            $closeSession = $conn->prepare('UPDATE `timeRecords` SET `jobDescription`= :jobDescription, `equipType`= :equipType, `unitNumber`= :unitNumber, `unitHours`= :unitHours,
                                `timeOut`= :timeOut WHERE `employeeID`= :employeeID AND `date`= :dateToday AND `timeOut` IS NULL');
            $closeSession->execute(array(
                ':employeeID' => $employeeID,
                ':timeOut' => $timeToday,
                ':dateToday' => $dateToday,
                ':jobDescription' => $jobDescription,
                ':equipType' => $equipType,
                ':unitNumber' => $unitNumber,
                ':unitHours' => $unitHours
            ));
            $putClockIn->execute(array(
                ':employeeID' => $employeeID,
                ':dateToday' => $dateToday,
                ':timeIn' => $timeToday
            ));
            echo "There was an update";
        } else {
            echo "There was no Match";
        }
    }
}

require 'summary.php';


How to avoid using exec() and eval() in Python I wrote a script to execute a service on the Forestry TEP platform via its REST API. This service has certain input parameters, some of them are numerical values, other strings or files. The current workflow is:

There is a configuration file that has options for file and literal inputs. Each option is a dictionary, where each key is equal to one parameter of the service I want to execute. Example of such file:
file_inputs= {"cfg" : "forest_flux/FTEP_service/cfg.ini",
              "parsawen" : "forest_flux/FTEP_service/parsawen.csv",
              "weather" : "forestflux/data/fin_01/weather/weather_2004.csv"}
literal_inputs = {"version" : 3}

In the script, I read the cfg and iterate over items in these dictionaries. For each key,value pair, I use exec() to store the value in a class variable of the same name. For file inputs, I first upload the file on the platform and store the file's location on the platform in the variable.
    input_files_dict = json.loads(cfg.get("service_info", "file_inputs"))
    for key, val in input_files_dict.items():
        exec("self.{} = self.post_data('{}')".format(key, val))

    literal_inputs_dict = json.loads(cfg.get("service_info", "literal_inputs"))
    for key, val in literal_inputs_dict.items():
        exec("self.{} = '{}'".format(key, val))

I request service inputs and I try to match the service parameters with my class variables. I add it to a dictionary of parameters which I then send to the platform when executing the service.
    r2 = requests.get(url = url2, verify=False, allow_redirects=False, cookies=self.session_cookies)
    data_inputs = json.loads(r2.text)['serviceDescriptor']['dataInputs']
    for _input in data_inputs:
        value_to_set = eval("self." + _input["id"].lower())
        job_config["inputs"][_input["id"]] = [value_to_set]

This current workflow works well for me but I have strong suspicion that it is a very bad practice. I have read that using exec() and eval() is discouraged as there are security issues. However, I cannot think of a better way to achieve this functionality, i.e. to pair values in configuration with service inputs automatically. I want this script to be generic, i.e. usable for any service on the platform.
Question: How could I do this without using the problematic functions? How could using these functions be problematic in this case?
Please note that I removed a lot of code that is not related to the problem I am asking about (e.g. error handling).

You can use getattr, setattr and delattr to retrieve, modify and remove attributes respectively. This avoids most potential messes eval has.
input_files_dict = json.loads(cfg.get("service_info", "file_inputs"))
for key, val in input_files_dict.items():
    setattr(self, key, self.post_data(f'{value}'))


Reading data from multiple different resource bundle property files in Java I have class structure to read the data from resource bundle (property files).
import java.util.Locale;
import java.util.Objects;
import java.util.ResourceBundle;

/**
 * The class will be used to read the resource bundle to achieve localization
 * and internationalization
 *
 */
public class ResourceBundleReader {

  private String resourceBundle = null;

  /**
   * Constructor of the class
   *
   * @param resourceBundle
   * @throws NullPointerException
   *           if resourceBundle is null
   */
  public ResourceBundleReader(final String resourceBundle) {

    Objects.requireNonNull(resourceBundle);

    this.resourceBundle = resourceBundle;
  }

  /**
   * Return message by input key for default locale
   *
   * @param key
   * @return
   */
  public String getMessage(final String key) {

    return getMessage(key, Locale.getDefault());
  }

  /**
   * Return message by input key for input locale locale
   *
   * @param key
   * @return
   */
  public String getMessage(final String key, final Locale locale) {
    final ResourceBundle mybundle = ResourceBundle.getBundle(resourceBundle, locale);
    return mybundle.getString(key);
  }
}

As there are multiple type of resource bundles, I have created individual implementation for that. For example below two:
Exception Message:
public final class ExceptionMessageResolver {

  private static final String EXCEPTION_MESSAGES_RESOURCE = "exceptionmessages"; //$NON-NLS-1$

  private static final ResourceBundleReader RESOURCE_BUNDLE_READER = new ResourceBundleReader(
      EXCEPTION_MESSAGES_RESOURCE);

  /**
   * Private constructor to prevent instantiation Constructor of the class
   */
  private ExceptionMessageResolver() {
    super();
  }

  /**
   * return exception message by default locale
   *
   * @param key
   * @return
   */
  public static String getMessage(final String key) {

    return getMessage(key, Locale.getDefault());
  }

  /**
   * Return exception message for input locale
   *
   * @param key
   * @param locale
   * @return
   */
  public static String getMessage(final String key, final Locale locale) {
    return RESOURCE_BUNDLE_READER.getMessage(key, locale);
  }

}

Label Messages:
public final class ResourceLabelResolver {

  private static final String LABEL_RESOURCE = "resourcelabels"; //$NON-NLS-1$

  private static final ResourceBundleReader RESOURCE_BUNDLE_READER = new ResourceBundleReader(LABEL_RESOURCE);

  /**
   * Private constructor to prevent instantiation Constructor of the class
   */
  private ResourceLabelResolver() {
    super();
  }

  /**
   * return label message by default locale
   *
   * @param key
   * @return
   */
  public static String getMessage(final String key) {

    return getMessage(key, Locale.getDefault());
  }

  /**
   * Return label message for input locale
   *
   * @param key
   * @param locale
   * @return
   */
  public static String getMessage(final String key, final Locale locale) {
    return RESOURCE_BUNDLE_READER.getMessage(key, locale);
  }

}

Is there any more elegant way for this? Or any way to create abase class for my two readers to prevent duplicate methods in code?

I don't see the need to implement sub classes in your code example above. You need inheritance only if you add specific implementation on the sub class. All you need is to create an interface to encapsulate the resolver (IResourceBundle) for abstraction, then create a factory for them.
public interface IResourceBundle {
    String getMessage(final String key);
    String getMessage(final String key, final Locale locale);
}

public enum ResourceBundles {
    EXCEPTION_MESSAGE("exceptionmessages"),
    LABEL("resourcelabels");
    private String bundleName;
    private ResourceBundles(String name) {
        bundleName = name;
    }
    public String toString() {
        return bundleName;
    }
}

public final class ResourceResolverFactory {
    private Map<ResourceBundles, IResourceBundle> resolvers;
    private ResourceResolverFactory() {
        resolvers = new HashMap<ResourceBundles, IResourceBundle>();
    }
    public IResourceBundle getBundle(final ResourceBundles resourceBundle) {
        IResourceBundle resolver = resolvers.get(resourceBundle);
        if(resolver != null) {
            return resolver;
        }
        resolver = new ResourceBundleReader(resourceBundle.toString());
        resolvers.put(resourceBundle, resolver);
        return resolver;
    }
    private static final ResourceResolverFactory instance = new ResourceResolverFactory();
    public static ResourceResolverFactory getInstance() {
        return instance;
    }
}

public class ResourceBundleReader implements IResourceBundle {
    private String resourceBundle = null;
    public ResourceBundleReader(final String resourceBundle) {
        Objects.requireNonNull(resourceBundle);
        this.resourceBundle = resourceBundle;
    }
    public String getMessage(final String key) {
        return getMessage(key, Locale.getDefault());
    }
    public String getMessage(final String key, final Locale locale) {
        final ResourceBundle mybundle = ResourceBundle.getBundle(resourceBundle, locale);
        return mybundle.getString(key);
    }
}

public class Main {
    public static void main(String[] args) {
        ResourceResolverFactory factory = ResourceResolverFactory.getInstance();
        IResourceBundle exceptionMessageResolver = factory.getBundle(ResourceBundles.EXCEPTION_MESSAGES_RESOURCE);
        // do something with exceptionMessageResolver
        IResourceBundle labelResolver = factory.getBundle(ResourceBundles.EXCEPTION_MESSAGES_RESOURCE);
        // do something with labelResolver
    }
}

In this case, adding more resource bundle means adding new enum in ResourceBundles not introducing new sub-class.


Is there a more dynamic way to target siblings in CSS / SCSS? The code I'm working on looks like this - All the way up to 50... (Not ideal)
There can be anywhere from 1 to 100 anchor/div combinations, but more often than not in single digits so there's almost always a lot of redundant CSS rules.
I was wondering if there was a better way to write the CSS / SCSS for this rather than code for each possible outcome manually?  Or would the only way to do this be by using JS?
.content { display: none; }
.anchor-1:hover ~ .content-1,
.anchor-2:hover ~ .content-2,
.anchor-3:hover ~ .content-3,
.anchor-4:hover ~ .content-4,
.anchor-5:hover ~ .content-5 { display: block; }

I also have the code fiddled here: https://jsfiddle.net/rn437pw8/8/

There is! You can use :nth-child() with plenty of ways to select children elements of particular wrapper, for example odd or even elements, a range of elements, 3 elements after 4 other elements and repeat.
Check this generator to create a rule and help you visualize what elements will be selected:
https://css-tricks.com/examples/nth-child-tester/
Docs:
https://developer.mozilla.org/en-US/docs/Web/CSS/:nth-child


php one-time prepared statement execution function I use prepared statements often even when I only need to execute the statement once (for security), so I implemented a function to abstract away all the function calls on the mysqli_stmt object, as well as bind_param()'s first argument since as far as my tests show it works identically even when int parameters are marked as strings.

<?php
$conn = new mysqli('localhost', 'name', 'password', 'db');

if ($conn->connect_error)
    die('Connection to database failed: ' . $conn->connect_error);

function stmt($query, $params){

   array_unshift($params, str_repeat('s', sizeof($params)));
   for($i = 1; $i < sizeof($params); $i++){
      $params[$i] = &$params[$i];
   }

   $stmt = $GLOBALS['conn']->stmt_init();
   $stmt->prepare($query);
   $method = new ReflectionMethod('mysqli_stmt', 'bind_param');
   $method->invokeArgs($stmt, $params);
   $stmt->execute();

   if($stmt->error){
      $result = ['error' => $stmt->error];
   } else {
      $result = $stmt->get_result();
   }

   $stmt->close();
   return $result;
}
?>

Usage example:

<?php
   $result = stmt('SELECT * FROM table_name WHERE id IN(?,?,?)', [1,2,3]);
?>

I tested your code and it does work, so far, so good. This type of code has been written many times, so by searching online you can find a lot of good examples.

The main problem I have with your code is that it is rather difficult to understand. I can work it out, but it is not obvious. Starting with prepending the $params with something, then the very weird: $params[$i] = &$params[$i] loop, followed by the usage of ReflectionMethod normally used for reverse-engineering code.

I prefer more down to earth code for a simple function like this. Something like:

function executeQuery($mysqli, $query, $parameters)
{
    $stmt = $mysqli->stmt_init();
    if ($stmt->prepare($query)) {
        $types = str_repeat("s", count($parameters));
        if ($stmt->bind_param($types, ...$parameters)) {
            if ($stmt->execute()) {
                return $stmt->get_result();
            }
        }
    }
    return ['error' => $stmt->error];
}

Short and sweet. Some notes:

I try to use a function name that actually reflects what the function does.
I supply the database connection as an argument, for more flexibility. You can use multiple database connections and they don't need to be in the global scope.
I check whether the query could be properly prepared.
My code differs quite a bit from your code when it comes to binding the parameters. As you can see this is quite straightforward. Using ... to access variable arguments has been available since PHP 5.6 which came out in 2014.
By directly returning the results when the execution was successful I know that an error must have occurred when the last line of the function is executed. This therefore also catches other problems.

Personally I would not have expected to get a MySQLi result object out of this function. Because it will always have to be processed. Why not do this processing inside this function? Like this:

function executeQuery($database, $query, $parameters)
{
    $stmt = $database->stmt_init();
    if ($stmt->prepare($query)) {
        $types = str_repeat("s", count($parameters));
        if ($stmt->bind_param($types, ...$parameters)) {
            if ($stmt->execute()) {
                if ($result = $stmt->get_result()) {
                    $rows = [];
                    while ($row = $result->fetch_assoc()) {
                        $rows[] = $row;
                    }
                    return $rows;
                }
            }
        }
    }
    return ['error' => $stmt->error];
}

Now you simply get an array back. I agree that is not much different from returning a MySQLi result, but I am thinking ahead. Suppose you decide to change over from MySQLi to PDO in the future. You can easily recode the function above to work with PDO, but recoding the handling of MySQLi results everywhere in your code will be a lot harder. So I am using the function to abstract away from a particular database interface.

Some people don't like the deep nesting of if () {} blocks. To prevent this you could instead write something, like the code below, for all these blocks:

 if (!$stmt->prepare($query)) {
     return ['error' => $stmt->error];
 }

I have to repeat that there are lots of ways of doing this. The answer I gave is based on the code you presented. It is, for instance, not hard to find out the type of the parameters, and adjust the $types string accordingly.


Reading image files asynchronously and displaying them on the page I am trying to display images from a folder in my project using the "fs" module with NodeJS. I need to read all images in a directory and run them in a loop for them to be displayed. I was able to do it but I'm not sure if this is the proper or good way to do it.

I inserted my readdir (Reading Files Asynchronously) inside my Homepage route.
The images were displayed but I have no one to ask if I did it the proper way.

const express = require('express');
const router = express.Router();
const fs = require('fs');

router.get('/', function(){
    fs.readdir('./assets/images/', (err, files) => {
        if(err) {
            throw err;
        }
        res.render('home', {
            files: files
        });
    });
});

module.exports = router;

Don't use callbacks, it's hard to read and maintain, the fs.readdir function will return a promise if you didn't provide a callback function.
Use arrow functions when you could.
Respond with an error instead of throwing (unless you have a catch wrapper middleware).
Use property shorthands.
You forgot req and res.

Try this:

const express = require('express');
const router = express.Router();
const fs = require('fs');

router.get('/', async (req, res) => {
  try {
    const files = await fs.readdir('./assets/images/')
    res.render({ files })  // notice the property shorthand
  } catch (error) {
    res.status(404).json({ error })
  }
});

module.exports = router;


Compact jQuery tool tip code, without jQueryUI I have this tooltip code that I came up with, using jQuery:

$('div.a')
.bind('mousemove', function(e){
    $('#follow').css({
       left:  e.pageX + 20,
       top:   e.pageY
    });
})
.mouseenter(function() {
    $('#follow').css('display','block');
})
.mouseleave(function() {
    $('#follow').css('display','none');
})

I was wondering if there is anyway to compact it even more than it already is (ignore whitespace).

Here is a jsFiddle, if you'd like to test it.

Bind method should be used only when if your element is not loaded into DOM on pageload. So you could attach events to it using bind after wards. also if your using jquery >= 1.7 use 'On' method to achive it. but as per your js fiddle you don't need it at all.

second you could declare a variable to store the jquery object of follow. Make sure about the variable scope though.

var follow=$('#follow');

$('div.a').mousemove(function(e){
    follow.css({
       left:  e.pageX + 20,
       top:   e.pageY
    });
})
.mouseenter(function() {
    follow.css('display','block');
})
.mouseleave(function() {
    follow.css('display','none');
})


Code review of code to strip 0 from decimal series The following code would strip 0 (all numbers ending with 0, it can contain 0, eg: 105 can be is valid but 150 should be eliminated) and return a number in decimal series if all numbers ending with 0 were stripped.
 Example : 0 -> 1, 10 -> 11, 19 -> 22.
I would just appreciate any code reviews.

  public static int getStrippedNumber(int num) {
    int setId = (num / 10) + 1;
    int newNum = num + setId;
    int newNumSetId = (newNum / 10) + 1;
    int numberToReturn = num + newNumSetId;
    return numberToReturn % 10 ==0 ? numberToReturn + 1 : numberToReturn;
    }

Well... Sorry to say this, but it's crappy!

The function name doesn't explain what the function does (and using your explanation above for Javadoc would only make things worse, judging from the confusion in the comments)
Even knowing the implementation doesn't help, it's hard to tell if the steps performed inside the function correspond to a real-world concept or represent some algorithm
it's not clear if this function makes sense for negative values, I'm pretty sure it would return unexpected values (i.e. output for x and -x would be drastically different). If the function is not defined for negative values, throw an IllegalArgumentException when num < 0
You use confusing names for variables; I think even a, b, c etc. would be better than num, newNum, numberToReturn (nb. this last one is additionally misleading as you still process it before returning)
The function seems to solve some general problem but it uses arbitrary depth: repeats the same operation twice (in lines 1&2 and 3&4) instead of using a loop or recursion (which would also communicate your intentions better)

I know this is the part where you give your recommendations but I believe this function is broken beyond repair until you make it clear what its purpose and contract are. Remember that most real-world mathematical problems are already solved and have well-described algorithms, using well-tested solutions is always a better idea than trying to reinvent the wheel.


Remove duplicates from string without using additional buffer I wrote a simple program to remove duplicates from a String without using additional buffer.

The program should filter the duplicates and return just the unique string.

Example:

  Input:
  FOOOOOOOOLLLLLOWWWWWWWWWW UUUUP

  Output:
  FOLW UP

I just want to know if the below solution is a good solution for my problem statement.

public class compareString {
    public static void main(String args[]) {
        removeDuplicateString("FOOOOOOOOLLLLLOWWWWWWWWWW UUUUP");
    }

    public static void removeDuplicateString(String input) {
        String value1 = input;
        String value2 = input;
        String finalValue = "";
        int count = 0;
        char char1;
        char char2 = 0;
        for (int i = 0; i < value1.length(); i++) {
            flag = 0;
            char1 = value1.charAt(i);
            for (int j = 0; j < value2.length(); j++) {
                char2 = value2.charAt(j);
                if (char1 == char2) {
                    count++;
                }
            }

            if (count > 1) {
                finalValue=finalValue+char1;
                i=i+(count-1);
            } else {
                finalValue = finalValue + char1;
            }
            count = 0;
        }
        System.out.println(finalValue);
    }
}

Thats not working. Try the input: abaa (result: aa)

I do not know what you mean with "without using additional buffer", because finalValue is at least an additional buffer (which is kind of necessary. You could create a view, but this will be a bit complex).

An easier approach would be to create a LinkedHastSet. Insert all characters, output the set, done:

public static void removeDuplicateString2(final String input) {
    final Set<Character> set = new LinkedHashSet<>();
    for (int i = 0; i < input.length(); i++)
        set.add(input.charAt(i));
    final StringBuilder stringBuilder = new StringBuilder(set.size());
    for (final Character character : set)
        stringBuilder.append(character);
    System.out.println(stringBuilder);
}

About your code:

removeDuplicateString

I would call it something like removeMultipleOccurrence

String finalValue = "";

You may use a StringBuilder to reduce string concatenations.

    String value1 = input;
    String value2 = input;

You do not need these variables, just use input instead of value1/2

    int count = 0;
    char char1;
    char char2 = 0;

You do not need char2, and you can put the rest inside the loop to the initializing place.

        flag = 0;

This variable is not defined.

        if (count > 1) {
            finalValue=finalValue+char1;
            i=i+(count-1);
        } else {
            finalValue = finalValue + char1;
        }

You may move the finalValue line from the if/else. Then you can skip the else part, because it is empty.

If you combine everything, you could have:

public static void removeMultipleOccurrence(final String input) {
    StringBuilder result = new StringBuilder();
    for (int i = 0; i < input.length(); i++) {
        int count = 0;
        final char char1 = input.charAt(i);
        for (int j = 0; j < input.length(); j++) {
            if (char1 == input.charAt(j)) {
                count++;
            }
        }

        if (count > 1)
            i = i + (count - 1);
        result.append(char1);
    }
    System.out.println(result);
}

Hint: This is still not working, just doing the same in a more clear way.
If you want to do it in your way, you have to search the string before the current position if you have already the current char.

This could be done in this way:

public static void removeMultipleOccurrence(final String input) {
    String finalValue = "";
    for (int i = 0; i < input.length(); i++) {
        int count = 0;
        final char currentChar = input.charAt(i);
        for (int j = 0; j < i; j++) {
            if (currentChar == input.charAt(j)) {
                ++count;
            }
        }

        if (!(count > 0))
            finalValue = finalValue + currentChar;
    }
    System.out.println(finalValue);
}

If we look at the count, it is just a boolean, so we end up with:

public static void removeMultipleOccurrence(final String input) {
    final StringBuilder result = new StringBuilder();
    for (int i = 0; i < input.length(); i++) {
        boolean alreadySeen = false;
        final char currentChar = input.charAt(i);
        for (int j = 0; j < i; j++) {
            if (currentChar == input.charAt(j)) {
                alreadySeen = true;
                break;
            }
        }

        if (!alreadySeen)
            result.append(currentChar);
    }
    System.out.println(result);
}

We could get rid of the boolean now:

public static void removeMultipleOccurrence(final String input) {
    final StringBuilder result = new StringBuilder();
    withNextChar: for (int i = 0; i < input.length(); i++) {
        final char currentChar = input.charAt(i);
        for (int j = 0; j < i; j++) {
            if (currentChar == input.charAt(j))
                continue withNextChar;
        }
        result.append(currentChar);
    }
    System.out.println(result);
}

Or, we could look in the result, if we have it already:

public static void removeMultipleOccurrence(final String input) {
    final StringBuilder result = new StringBuilder();
    for (int i = 0; i < input.length(); i++) {
        String currentChar = input.substring(i, i + 1);
        if (result.indexOf(currentChar) < 0) //if not contained
            result.append(currentChar);
    }
    System.out.println(result);
}


Fast Document Distance, C++ This is an attempt at a fast "document distance" implementation in C++. For reference, I found this problem in MIT's OpenCourseware 6.006 algorithms course.

A quick look at the callgrind output shows that my performance killers are, in order:

the transfrom call in formatToken
Constructing inputString in wordOccurences
Indexing into the unordered_map (calling operator[])
reading from the stringstream in order to split the string

Here's the code:

#include <algorithm>
#include <fstream>
#include <functional>
#include <iostream>
#include <iterator>
#include <sstream>
#include <string>
#include <unordered_map>

using namespace std;

string const g_source      = "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz1234567890";
string const g_destination = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890";

bool containsKey(unordered_map<string, int> const &wordMap, string const &key)
{
    return wordMap.find(key) != wordMap.end();
}

template <typename ForwardIterator>
unordered_map<char, char> translationTable(
    ForwardIterator first,
    ForwardIterator last,
    ForwardIterator dFirst)
{
    unordered_map<char, char> table;

    for(; first != last; ++first, ++dFirst) {
        table[*dFirst] = *first;
    }

    return table;
}

void formatToken(string &rawString)
{
    static unordered_map<char, char> charTranslationTable = translationTable(
        g_source.begin(),
        g_source.end(),
        g_destination.begin());

    transform(
        rawString.begin(),
        rawString.end(),
        rawString.begin(),
        [](char c) { char ret = charTranslationTable[c]; return ret ? ret : ' '; });
}

unordered_map<string, int> wordOccurences(ifstream &is)
{
    unordered_map<string, int> result;
    string inputString{istreambuf_iterator<char>(is), istreambuf_iterator<char>()};
    formatToken(inputString);

    istringstream stream(inputString);
    string token;

    while (stream) {
        stream >> token;
        result[token] += 1;
    }

    return result;
}

double documentMagnitude(unordered_map<string, int> const &document)
{
    double magnitude = 0.0;

    for (auto const &word : document) {
        magnitude += double(word.second * word.second);
    }

    return sqrt(magnitude);
}

int main(int argc, char *argv[])
{
    string filename1 = arg[1];
    string filename2 = arg[2];

    ifstream file1(filename1);
    ifstream file2(filename2);

    unordered_map<string, int> document1 = wordOccurences(file1);
    unordered_map<string, int> document2 = wordOccurences(file2);

    double magnitude1 = documentMagnitude(document1);
    double magnitude2 = documentMagnitude(document2);

    double innerProduct = 0.0;

    for (auto const &word : document2) {
        if (containsKey(document1, word.first)) {
            innerProduct += double(word.second * document1[word.first]);
        }
    }

    double cosAngle = innerProduct / magnitude1 / magnitude2;

    cout << "Distance: " << acos(cosAngle) << '\n';
}

I'm running at speeds comparable to the CPython implementation from the aforementioned course. If I try to pre-allocate the string size to the stream-size, I get less than 1% improvement in performance.

Speed/optimization

Unfortunately, istream_buf_iterators aren't a particularly fast way to read the entirety of a file into a string. One simple way to improve speed is to use the istream's read instead. To do that, you can replace this:

string inputString{istreambuf_iterator<char>(is), istreambuf_iterator<char>()};

...with something like this:

string inputString;

is.seekg(0, std::ios::end);
size_t length = is.tellg();
inputString.resize(length);
is.seekg(0, std::ios::beg);
is.read(&inputString[0], length);

At least in my testing, this reduces the time from around 800 ms to around 600 ms for a pair of test files I threw together (about 6.5 megabytes, mostly from project Gutenberg, with one chapter of one book edited out of one of the files).

If you're running this under Linux, you'll probably also benefit from adding a line like this toward the beginning of main:

std::ios::sync_with_stdio(false);

On Windows this has essentially no effect, but on Linux it can make quite a bit of difference.

Correctness

Command line arguments

You declare the command line arguments as argc/argv like usual, but attempt to refer to two arguments as arg[1] and arg[2] (should be argv instead of arg).

You should also check that argc > 2 before attempting to use argv[2].

EOF detection

This code:

while (stream) {
    stream >> token;
    result[token] += 1;
}

...is broken. It doesn't detect the end of the file correctly. For accurate EOF detection, consider using something like this instead:

while (stream >> token)
    ++result[token];

Translation

I'm pretty sure you transposed these two strings:

string const g_source      = "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz1234567890";
string const g_destination = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890";

I'm pretty sure the intent was the either upper-case or lower-case input would be mapped to lower-case output. As it stands, you seem to be trying to map lower-case input to both lower-case and upper-case output.

Style

Simplicity

Although it probably won't make a huge difference in speed, I'd rather use a simple std::array for the translation table, and just fill in the result character for every possible input:

static const size = std::numeric_limits<unsigned char>::max();

std::array<char, size> translation_table;

for (int i=0; i<size; i++)
   if (std::isalpha(i))
       translation_table[i] = std::tolower(i);
   else if (std::isdigit(i))
       translation_table[i] = i;
   else
       translation_table[i] = ' ';

Then translating a character becomes utterly trivial:

transform(
    rawString.begin(),
    rawString.end(),
    rawString.begin(),
    [](unsigned char c) { return translation_table[c]; });

Also note the change to unsigned char, so if we have an input that's negative when viewed as a char, we don't try to index outside the table.

Encapsulation

Taking the next step from the last point, I'd rather hide the implementation of the translation a bit more by wrapping it up into a class. For example, something like this might be usable:

class formatter {
    class xlat_table {
        static const int table_size = std::numeric_limits<unsigned char>::max();
        std::array<char, table_size> table;
    public:
        xlat_table() {
            for (int i = 0; i < table_size; i++)
                if (std::isalpha(i))
                    table[i] = std::tolower(i);
                else if (std::isdigit(i))
                    table[i] = i;
                else table[i] = ' ';
        }
        char operator[](size_t n) { return table[n]; }
    };

    static xlat_table table;

public:

    void operator()(std::string &rawString) {
        transform(
            rawString.begin(),
            rawString.end(),
            rawString.begin(),
            [this](unsigned char c) { return table[c]; });
    }
};

formatter::xlat_table formatter::table;

This creates the xlat_table as a static member of formatter, so we can create formatter objects freely, without initializing a new translation table every time.

Conclusion

This should improve speed at least a little bit, but this will probably be I/O bound in most cases, so just translating the code to C++ isn't necessarily going to give any huge speed improvement. On the other hand, if you have something that can do really fast I/O (e.g., a reasonably high-end SSD) you might be more likely to see substantial gains.

My final code putting the pieces together looks like this:

#include <algorithm>
#include <fstream>
#include <functional>
#include <iostream>
#include <iterator>
#include <sstream>
#include <string>
#include <unordered_map>
#include <chrono>
#include <array>
#include <cctype>

using namespace std;

bool containsKey(unordered_map<string, int> const &wordMap, string const &key)
{
    return wordMap.find(key) != wordMap.end();
}

class formatter {
    class xlat_table {
        static const int table_size = std::numeric_limits<unsigned char>::max();
        std::array<char, table_size> table;
    public:
        xlat_table() {
            for (int i = 0; i < table_size; i++)
                if (std::isalpha(i))
                    table[i] = std::tolower(i);
                else if (std::isdigit(i))
                    table[i] = i;
                else table[i] = ' ';
        }
        char operator[](size_t n) { return table[n]; }
    };

    static xlat_table table;

public:

    void operator()(std::string &rawString) {
        transform(
            rawString.begin(),
            rawString.end(),
            rawString.begin(),
            [this](unsigned char c) { return table[c]; });
    }
};

formatter::xlat_table formatter::table;

unordered_map<string, int> wordOccurences(istream &is)
{
    unordered_map<string, int> result;

    string inputString;

    is.seekg(0, std::ios::end);
    size_t length = is.tellg();
    inputString.resize(length);
    is.seekg(0, std::ios::beg);
    is.read(&inputString[0], length);

    formatter()(inputString);

    istringstream stream(inputString);
    string token;

    while (stream >> token) {
        ++result[token];
    }

    return result;
}

double documentMagnitude(unordered_map<string, int> const &document)
{
    double magnitude = 0.0;

    for (auto const &word : document) {
        magnitude += double(word.second * word.second);
    }

    return sqrt(magnitude);
}

double cos_dist(std::istream &file1, std::istream &file2) {
    using namespace std::chrono;

    auto begin = high_resolution_clock::now();

    unordered_map<string, int> document1 = wordOccurences(file1);
    unordered_map<string, int> document2 = wordOccurences(file2);

    double magnitude1 = documentMagnitude(document1);
    double magnitude2 = documentMagnitude(document2);

    double innerProduct = 0.0;

    for (auto const &word : document2) {
        if (containsKey(document1, word.first)) {
            innerProduct += double(word.second * document1[word.first]);
        }
    }

    double cosAngle = innerProduct / magnitude1 / magnitude2;

    auto end = high_resolution_clock::now();

    std::cout << "Time: " << duration_cast<milliseconds>(end - begin).count() << " ms\n";

    return cosAngle;
}

int main(int argc, char *argv[])
{
    std::ios::sync_with_stdio(false);

    string filename1 = argv[1];
    string filename2 = argv[2];

    ifstream file1(filename1);
    ifstream file2(filename2);

    double cosAngle = cos_dist(file1, file2);

    cout << "Distance: " << acos(cosAngle) << '\n';
}

With all the pieces put together, this reduces the time (on my test files) from about 800 milliseconds to about 450-475 milliseconds. I haven't done any detailed profiling to find what parts of the code are consuming that time, so there might be some fairly obvious places to get more improvement.