Animated scrolling to a place in the same webpage I'm just beginning web design, and I've come up with this code to scroll to a place in the same page.

I don't know if this can or should be improved much longer, so I'm asking for any tips that can make this code better.

I know it's messy, but I will clean it later.  I just want to know if there's a better method.

$(document).ready(function(){
$('#Tecnologia').click(function(){
$(".ConHome").css('transition','all 0.4s ease');
$(".ConHome").css('transform','translateY(-50em)');
$("#BlankSpaceMenu").css('transition','all 0.4s ease');
$('#BlankSpaceMenu').css('transform','translateY(-50em)');
$(".ConTech").css('transition','all 0.4s ease');
$(".ConTech").css('transform','translateY(-50em)');
$(".TopHeader").css('transition','all 0.4s ease');
$(".TopHeader").css('border-bottom-color','rgba(0,0,255,1)');
})})
#BlankSpaceMenu{
position:relative;
height:7em;
width:100%;
background-color:rgba(0,0,255,1);
z-index:9;
}
.Container{
position:relative;
color:rgba(255,255,255,1);
}
.TopHeader{
 position:fixed;
background-color:rgba(255,153,0,1);
color:rgba(255,255,255,1);
height:7em;
width:100%;
z-index:10000;
border-bottom:rgba(0,0,0,1) dashed 10px;
}
.ConHome{
position: relative;
background-color: rgba(0,0,0,1);
color: rgba(255,255,255,1);
height: 50em;
width: 100%;
z-index:10;
}
.ConTech{
position: relative;
background-color:rgba(0,0,255,1);
color:rgba(255,255,255,1);
height: 50em;
width: 100%;
z-index:8;
}

Tecnologia

....

...

You should declare these styles into classes, and use jQuery to toggle the classes. That way, you have good "separation of concerns". You end up with a smaller base code.

Additionally, one should not use the all for transition. It's bad for performance. Ideally, one should just indicate what property to transition.

There's also this neat trick that forces CSS to be hardware-accelerated. However, you should use it carefully. I read that there's drawbacks to excessive use, and instead of performance gains, you lose performance.

With what I know, here's how you can optimize that animation code:

CSS:

#BlankSpaceMenu.transition,
.ConHome.transition,
.ConTech.transition,
.TopHeader.transition{
  transition : all 0.4 ease
}

#BlankSpaceMenu.transform,
.ConHome.transform,
.ConTech.transform{
  transform : translateY(-50em)
}

.TopHeader.transform{
  border-bottom-color : rgba(0,0,255,1)
}

JS:

$(function () {

  // Cache the set, assuming they don't change.
  var animationSet = $('.ConHome, #BlankSpaceMenu, .ConTech, .TopHeader');

  $('#Tecnologia').click(function () {
    animationSet.addClass('transition transform');
  });

});


Name filtering in Java I have a search box for searching for people in a list of names.
Here is the code that runs every time the query changes.  I could not find anything open-sourced for this, so I wanted to make sure this looks good and efficient.

How does it look?

private final List<String> mTotalNames = ............ // some ArrayList<String> with all the names
private final List<String> mAvailableNames = new ArrayList<String>(mTotalNames);

private void doOnQueryTextChange(final String s) {
    mAvailableNames.clear();
    if (s.length() <= 0) {
        mAvailableNames.addAll(mTotalNames);
    } else {
        final String query = s.toLowerCase();
        for (String name : mTotalNames) {
            name = name.toLowerCase();
            final List<String> partNameList
                    = new LinkedList<String>(Arrays.asList(name.split("\\s+")));
            if (helper0(query, partNameList)) {
                mAvailableNames.add(user);
            }
        }
    }
    // yeah, out mAvailableNames is all updated
}

private static boolean helper0(final String query, final List<String> partNameList) {
    for (String partQuery : query.split("\\s+")) {
        if (!helper1(partQuery, partNameList)) {
            return false;
        }
    }
    return true;
}

private static boolean helper1(final String partQuery, final List<String> partNameList) {
    if (partQuery.trim().length() <= 0) return true;
    final Iterator<String> iterator = partNameList.iterator();
    while (iterator.hasNext()) {
        final String partName = iterator.next();
        if (partName.startsWith(partQuery)
                || partName.trim().length() <= 0) {
            iterator.remove();
            return true;
        }
    }
    return false;
}

Note: The concepts presented in this answer have been implemented in the following 'alternate' implementation in this question: Name Filtering using a Trie

Interesting problem. Your code is hard to read, not because it is unstructured, or messy, but because the helper function names are hard to grasp.... what do they do?

So, on the good side, your 'visual' style is consistent, and neat. All your lines are indented well, and braced, etc. Well, except for this line:

if (partQuery.trim().length() <= 0) return true;

Naming

The first issue is naming..... Four things:

helper0 and helper1 .... do not help, despite the name.
Not a big deal, it's probably a 'local rule', but it is unconventional to use 'prefixes' in Java for variable names. What does the m mean? mTotalNames should just be totalNames. You are using "Hungarian" notation of some sort, and that is not good practice in Java
'Part' is overloaded in too many places. partQuery, partName, partNameList, it becomes hard to remember which variables are which.
iterator is a horrible name. We know it is an iterator, but what does it iterate?

Algorithm

This is a situation where the right data structure will help a lot. You are doing and repeating too much work. Consider this:

you convert names to lower case often.
you split names.... often.
you get prefixes... often.

What you need is a better algorithm, and the right one, is to build a trie.

The trie will be indexed by the characters in the names. This will allow you to search for names in a very fast way, and not do any of the overhead processing repeatedly.
Build the trie once, and then use it repeatedly when you type to filter the names.

Bug

Your code will not process things correctly for a certain subset of names/queries.

Consider the name "Andy Anderson", and the search query "And Andy".... this will not match, because the search query word 'And' will match against 'Andy' first, and then that leaves the search term 'Andy', which will not match against the word 'Anderson'.

You need to sort the search terms in decreasing-length order, and always search with the longest query term first.

Follow Up

This subject intrigues me, I may implement my recommended solution myself... in a day or so.


Creating a string of random characters I'm generating domains with random names. I used the map function because it packages the results into a list which I can then join() into a string but the way I used the lambda function could be confusing to people reading this later.

def generateHost():
  charSet = 'abcdefghijklmnopqrstuvwxyz1234567890'
  minLength = 5
  maxLength = 30
  length = random.randint(minLength, maxLength)

  return ''.join(map(lambda unused : random.choice(charSet), range(length)))+".com"

This function calls generateHost() 'size' times and creates an array of hosts. I could write it like the above but it would be the same problem. I wrote it with a for loop which I'm still not satisfied with.

def generateHostList(size):
  result = []
  for i in range(size):
    result += [generateHost()]
  return result

The answer to this question suggests the following:

''.join(random.choice(string.ascii_lowercase + string.digits) for _ in range(length))

It's close to your solution, but avoids having to type out the whole alphabet by using predefined values and avoids the use of map() and lambda.


Java synchronous queue implementation I just want to make sure there aren't any deadlocks or inconsistencies (the code is also available on GitHub).

Disclaimer - In real life, I would not implement a queue myself, but use an existing implementation. This is just preparations for job interviews.

package org.basic.concurrent;

import java.util.ArrayList;
import java.util.List;

public class SynchedQueue<T> {
    private final Object lock = new Object();
    private final Object full = new Object();
    private final Object free = new Object();

    private final List<T> buffer;
    private int head;
    private int tail;
    private int size;
    private final int capacity;

    public SynchedQueue(int capacity) {
        // http://stackoverflow.com/questions/4912088/how-to-create-a-fixed-size-generic-buffer-in-java
        buffer = new ArrayList<T>(capacity);
        for (int i = 0; i < capacity; ++i)
            buffer.add(null);
        this.capacity = capacity;
    }

    public void enqueue(T x) {
        if (x == null)
            throw new RuntimeException("Doesn't allow storing nulls");

        synchronized (lock) {
            while (!tryEnqueue(x)) {
                try {
                    free.wait();
                } catch (InterruptedException ignored) {
                }
            }
            full.notify();
        }
    }

    public T dequeue() {
        synchronized (lock) {
            while (true) {
                T item = tryDequeue();
                if (item != null)
                {
                    free.notify();
                    return item;
                }
                try {
                    full.wait();
                }
                catch (InterruptedException ignored) {}
            }
        }
    }

    private boolean tryEnqueue(T x) {
        assert size <= capacity;
        if (size >= capacity) {
            return false;
        }

        buffer.set(tail, x);
        tail = (tail + 1) % capacity;
        ++size;
        return true;
    }

    private T tryDequeue() {
        assert size >= 0;
        if (size == 0)
            return null;
        T item = buffer.get(head);
        head = (head + 1) % capacity;
        --size;
        return item;
    }
}

I have not coded in Java for a few years so it might be good to get another opinion, though some points did stand out while reading your code.

It seems odd to me that you would use an ArrayList with a fixed size, personally I would just use an array for the buffer.
when you dequeue, you do not overwrite the value until you enqueue over the top of it. This might not seem like an issue (and if you only queue primative types like int, its not) but if you use reference types and the queue cycles slowly then it means you hold a reference to the object for longer than needed and it cannot be collected.
I'm a bit worried about catching the InterruptedException inside a while(true) loops without breaking out of the loop. The most common reason I have seen for one thread to interrupt another is when the interrupting thread wants to give the interrupted thread the opportunity to terminate gratefully, but this will prevent that option. I thought about what can be returned in such a case and eventually decided it might be best to let this exception flow through and be handled by the caller.


Finding the closest point to a list of points I'm trying to find the closest point (Euclidean distance) from a user-inputted point to a list of 50,000 points that I have. Note that the list of points changes all the time. and the closest distance depends on when and where the user clicks on the point.

#find the nearest point from a given point to a large list of points

import numpy as np

def distance(pt_1, pt_2):
    pt_1 = np.array((pt_1[0], pt_1[1]))
    pt_2 = np.array((pt_2[0], pt_2[1]))
    return np.linalg.norm(pt_1-pt_2)

def closest_node(node, nodes):
    pt = []
    dist = 9999999
    for n in nodes:
        if distance(node, n) <= dist:
            dist = distance(node, n)
            pt = n
    return pt

a = []
for x in range(50000):
    a.append((np.random.randint(0,1000),np.random.randint(0,1000)))

some_pt = (1, 2)

closest_node(some_pt, a)

It will certainly be faster if you vectorize the distance calculations:

def closest_node(node, nodes):
    nodes = np.asarray(nodes)
    dist_2 = np.sum((nodes - node)**2, axis=1)
    return np.argmin(dist_2)

There may be some speed to gain, and a lot of clarity to lose, by using one of the dot product functions:

def closest_node(node, nodes):
    nodes = np.asarray(nodes)
    deltas = nodes - node
    dist_2 = np.einsum('ij,ij->i', deltas, deltas)
    return np.argmin(dist_2)

Ideally, you would already have your list of point in an array, not a list, which will speed things up a lot.


Extraction of numbers from currency string I have the following code; whereas currency and views are in reality many JSON objects from an SAP export, that I can not rely upon to be in the same format. That is why I'm regexing over these inputs. My code looks clumsy to me. Can it be improved?

var currency = "20.002,03 €"; // sometimes this String is "-" or " "
var views    = "38";          // sometimes this String is "-" or " "

var reDigit = /[\d\,\.]+/g;
var map = {',':'.', '.':''};

if (typeof currency != 'undefined' && typeof views != 'undefined') {

    currency = currency.match(reDigit);
    views = views.match(reDigit);

    if (currency != null && views != null ) {

        currency = currency[0];
        views = views[0];

        currency = currency.replace(/[,.]/g, function(k) {
            return map[k];
        });
        return currency / views;
    } else {
        return undefined;
    };
} else {
    return undefined;
}

In the currency variable, dots need to get eliminated and commas turned into floating points. I need to cater for cases when currency and views variables do not have a number in their string or do have a dash or a blank in them.

Would you say this script can be optimsed to be more efficient?

Simplify the overall logical flow

Notice the multiple else conditions in every if statement when some required condition is not true:

if (...) {
    if (...) {
        return currency / views;
    } else {
        return undefined;
    }
} else {
    return undefined;
}

These else blocks are repetitive.
It would be better to think of the logic as multiple levels of checks,
where you only return after all checks passed,
reaching the innermost level.
If at any checks fail,
execution drops out of the nested if blocks and fall back to the return statement of the function at the end, like this:

if (...) {
    if (...) {
        return currency / views;
    }
}
return undefined;

More readable without the map

Instead of this:

currency = currency.replace(/[,.]/g, function(k) {
    return map[k];
});

I would find it more readable to use two replace calls instead:

currency = currency.replace(/\./g, '');
currency = currency.replace(/,/g, '.');

You could even chain them together,
but that would be less readable.

Another reason why this is more readable is that in the original solution,
the map is defined far away from where it's used.
That can invite mistakes,
such as adding patterns in map but forgetting to adjust the regex in the replace call,
or the other way around.

Naming

reDigit is not a good name for /[\d\,\.]+/g.
The word "digit" suggests \d (0 ... 9), but you have more there.
How about reMoney or reMoneyDigits instead?
That would make it clear that it's more than just digits.

Validation

Are you sure all inputs are valid?
The code will work and return values for clearly bad input too, such as:

var currency = "hello20.002,03 €";
var currency = "20.0.0..2,03 €";
var currency = "20.a0b0c2d,03 €";
var currency = "20.002,03... €";

For the last example it will return NaN.

Will all this be ok in your application?

Even if you think such inputs are impossible, I think you should check for them.
One day when you might corrupted inputs,
which, in the absence of solid defenses could propagate in your systems and manifest as strange problems that are extremely difficult to debug.
It's better to catch problems early,
as close to their source as possible.

The only way you can assume the inputs are always valid,
if they have been prevalidated before reaching this code.
And in that case, this code should be in a private method,
only callable from the place where the validation was performed,
to guarantee that it can't be called with invalid data.

Dividing strings

I noticed a bit late, but curiously, the code is actually dividing strings:

'123' / 3
// -> 41

Ah, sweet JavaScript, only you can be capable of such madness.

You should use parseFloat for currency, and perhaps parseInt for view,
to make them proper numbers.
Note that this will change some things.
For example:

'123..' / 3
// -> NaN
parseFloat('123..') / 3
// -> 41


LeetCode: Leaf-similar trees C# Please comment about performance

https://leetcode.com/problems/leaf-similar-trees/

  Consider all the leaves of a binary tree.  From left to right order,
  the values of those leaves form a leaf value sequence.

  For example, in the given tree above, the leaf value sequence is (6,
  7, 4, 9, 8).

  Two binary trees are considered leaf-similar if their leaf value
  sequence is the same.

  Return true if and only if the two given trees with head nodes root1
  and root2 are leaf-similar.

  Note:

  Both of the given trees will have between 1 and 100 nodes.

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

namespace TreeQuestions
{
    /// <summary>
    /// https://leetcode.com/problems/leaf-similar-trees/
    /// </summary>
    [TestClass]
    public class LeafSimilarTrees
    {
        [TestMethod]
        public void LeafSimilarTreesTest()
        {
            TreeNode root1 = new TreeNode(3);
            root1.left = new TreeNode(5);
            root1.left.left = new TreeNode(6);
            root1.left.right = new TreeNode(2);
            root1.left.right.left = new TreeNode(7);
            root1.left.right.right = new TreeNode(4);
            root1.right = new TreeNode(1);
            root1.right.left = new TreeNode(9);
            root1.right.right = new TreeNode(8);

            TreeNode root2 = new TreeNode(3);
            root2.left = new TreeNode(1);
            root2.left.left = new TreeNode(6);
            root2.left.right = new TreeNode(2);
            root2.left.right.left = new TreeNode(7);
            root2.left.right.right = new TreeNode(4);
            root2.right = new TreeNode(5);
            root2.right.left = new TreeNode(9);
            root2.right.right = new TreeNode(8);
            Assert.IsTrue(LeafSimilar(root1,root2));

        }

        public bool LeafSimilar(TreeNode root1, TreeNode root2)
        {
            List<int> tree1 = new List<int>();
            List<int> tree2 = new List<int>();
            DFS(root1, tree1);
            DFS(root2, tree2);
            if (tree1.Count != tree2.Count)
            {
                return false;
            }
            for (int i = 0; i < tree1.Count; i++)
            {
                if (tree1[i] != tree2[i])
                {
                    return false;
                }
            }
            return true;
        }

        public void DFS(TreeNode root, List<int> list)
        {
            if (root == null)
            {
                return;
            }
            if (root.left != null)
            {
                DFS(root.left, list);
            }
            if (root.right != null)
            {
                DFS(root.right, list);
            }
            if (root.left == null && root.right == null)
            {
                list.Add(root.val);
            }
        }
    }
}

There's a problem with your approach: it always visits all nodes to build value lists, even when the trees aren't similar at all. By obtaining leaf-node values lazily, you'll prevent a lot of unnecessary traversal work if the trees are different.

This can be achieved by changing DFS to a generator method (using yield), and comparing the results with SequenceEqual, as janos already pointed out. You'll want to use an explicit stack instead of recursion though, to avoid incurring too much overhead.

However, for trees that are leaf-similar, the added overhead ends up making things slower. The fastest approach is probably a recursive method that does a depth-first traversal of both trees at the same time, bailing out as soon as it finds a leaf-node difference.


Parsing travel duration data I'm trying to parse an estimated travel duration and distance between two waypoints using Google Directions API. Tap on this to see an example of data received.

private class func parseTravelData(
    data: AnyObject!,
    mode: ESTMode,
    completion: (NSDictionary?) -> Void) {
        var receivedData = data as? NSDictionary

        if let routes = receivedData?["routes"] as? NSArray {
            if routes.count > 0 {
                if let legs = routes[0]["legs"] as? NSArray {
                    if legs.count > 0 {

                        var dur: String? = nil

                        if let duration = legs[0]["duration"] as? NSDictionary {
                            if let v = duration["value"] as? Float {
                                if v > ConnectionManager().timeMax && mode == .Walking {
                                    completion(nil) // resend using drive mode
                                }
                                else {
                                    if let t = duration["text"] as? String {
                                        dur = t
                                    }
                                }
                            }
                        }

                        if let d = dur {
                            if let distance = legs[0]["distance"] as? NSDictionary {
                                if let dist = distance["text"] as? String {
                                    completion([
                                        "duration": d,
                                        "distance": dist])
                                }
                            }
                        }
                    }
                }
            }
        }
}

This code works fine but looks awful. What I need is do nothing if data is incorrect. I can't come anything up with it. The main goal is avoiding of getting different unwrapping errors if something is wrong. I would greatly appreciate any help.

Not sure if this code is correct or will compile but here is my attempt...

private class func parseTravelData(data: AnyObject!, mode: ESTMode, completion: NSDictionary? -> Void) {
    let receivedData = data as? NSDictionary
    let routes = receivedData?["routes"] as? NSArray
    let legs = routes?.firstObject?["legs"] as? NSDictionary

    if let firstLeg = legs?.firstObject as? NSDictionary {
        let firstLegDurationDict = firstLeg["duration"] as? NSDictionary
        let firstLegDuration = firstLegDurationDict?["value"] as? Float

        if firstLegDuration > ConnectionManager().timeMax && mode == .Walking {
            completion(nil) // resend using drive mode
        } else if let durationStr = firstLegDurationDict?["text"] as? String {
            let distanceDict = firstLeg["distance"] as? NSDictionary
            if let distanceStr = distanceDict?["text"] as? String {
                completion(["duration": durationStr, "distance": distanceStr])
            }
        }
    }
}

Here are a few things I did to try to make the code more concise

I noticed that the interesting stuff only happens if the route contains a leg. So I was able to eliminate a few if statement blocks by using optional chaining and moving code out of the if conditions and into the receivedData, routes, and legs variables. (The individual variables are not strictly necessary but make the code more readable). So the first "if let" check happens to see if there is actually a leg in the optional legs array.

Another optimization I made is that I moved the if let d = dur block into the else statement above since the code will only proceed if duration["text"] exists. This also allowed me to eliminate the var dur: String? = nil statement as well.

I also renamed some variables for more clarity, for example I renamed v to firstLegDuration.

I was able to eliminate the if let v block since if v (which is now firstLegDuration) is nil then v > ConnectionManager().timeMax will be false since nil > AnyObject == false.

One thing I noticed that the completion does not get called in all cases, which to me seems weird, so if there is an error or missing data then nothing will happen.


Output of 'ldd' to dictionary I want to use the output of the terminal command ldd:

root@kali-vm:~/Desktop# ldd simple
    linux-gate.so.1 =>  (0xb7722000)
    libcurl-gnutls.so.4 => /usr/lib/i386-linux-gnu/libcurl-gnutls.so.4 (0xb769c000)
    libc.so.6 => /lib/i386-linux-gnu/i686/cmov/libc.so.6 (0xb7539000)
    libidn.so.11 => /usr/lib/i386-linux-gnu/libidn.so.11 (0xb7505000)

And use it to build a dictionary that uses the library name as key and the path to the library as value. I've come up with this little Python snippet:

  # Get List of Librairies in Dictionary
  lddOut = subprocess.check_output(['ldd', fileName])
  libraries = {}
  for line in lddOut.splitlines():
    arr = line.split(" => ")
    library = arr[0].strip('\t')
    try:
      path = arr[1].split(' (0x')[0]
    except:
      pass
    libraries[library] = path

It works fine for my needs but I'm at the point where I don't want my code to just work but want it optimized and well structured/programmed. How can I improve this snippet?

EDIT: Note that I do not want the address (ie 0xbxxxxxx) in my dictionary

This kind of parsing by pattern matching is much better done using a regular expression.

import re

libraries = {}
for line in ldd_out.splitlines():
    match = re.match(r'\t(.*) => (.*) \(0x', line)
    if match:
        libraries[match.group(1)] = match.group(2)


Iterator-like interface for extracting records from a blob I have a blob whose format is something like:

(id1,key1,value1)(id2,key2,value2)....

where id is a uint32_t, and key and value are null-terminated strings. I am looking for an iterator-like interface that will extract records from the blob.

This is what I have come up with:

typedef struct blob_ {
    uint32_t size;  // length of the buffer
    char buf[];
}blob;

typedef struct record_ {
    uint32_t id;    // id
    char *key;      // pointer to key in the blob
    char *value;    // pointer to value in the blob
    char *next;    // pointer to where the next record starts
} record;

static record
get_next_obj(const char* begin, const char* end)
{
    record obj;
    if (begin == end) {
        obj.id = 0;
        obj.key = NULL;
        obj.value = NULL;
        obj._next = NULL;
    } else {

        memcpy(&obj.id, begin, sizeof(uint32_t));
        begin += sizeof(uint32_t);

        obj.key = begin;
        while(begin < end && *begin != '\0')
            ++begin;

        ++begin;

        obj.value = begin;
        while(begin < end && *begin != '\0')
            ++begin;
        ++begin;

        obj.next = begin;
    }
    return obj;
}

record
blob_list_next(const blob *data, const record *prev)
{
    const char *begin, *end;

    begin = prev->next;
    end = data->buf + data->size;

    return get_next_obj(begin, end);
}

record
blob_list_begin(const blob *data)
{
    const char *begin, *end;

    begin = data->buf;
    end = data->buf + data->size;
    return get_next_obj(begin, end);
}

record
blob_list_end(const blob *data)
{
    record obj;
    obj.id = 0;
    obj.key = NULL;
    obj.value = NULL;
    obj.next = NULL;
    return obj;
}

Then the API will be accessed with something like this:

record obj;
record obj_end;
// comp is a comparison operator of two record objects
for (obj = blob_list_begin(data), obj_end = blob_list_end(data);
    comp(obj, obj_end) == false; obj = blob_list_next(data, &obj))  {
   // access the obj
}

I used return-by-value semantics because I wanted to avoid calling malloc(). There will be a maximum of 4096 records in a tuple.

Some random comments:

The behaviour of flexible array members are defined in the C99 standard, but undefined in C90. Because of that reason, you might want to add a compiler switch guard against C90 compilers:

#if !defined __STDC_VERSION__ || (__STDC_VERSION__ < 199901L)
  #error Code must be compiled as C99 or later.
#endif

Why is the data binary data char and not uint8_t? char may or may not be signed, depending on the compiler.
Consider inlining get_next_obj(). Since you return the struct by value, even good C compilers will generate a lot of overhead code from that. If you can weed out at least the internal return-by-value, the algorithm will be a bit faster.
Any reason why the record objects can't hold a prev pointer? That is, why can't they be a double-linked list instead of a single linked one? And on the topic of linked lists, you may want to consider changing the type of the next pointer:

typedef struct record_ {
    uint32_t id;     // id
    char *key;       // pointer to key in the blob
    char *value;     // pointer to value in the blob
    record_ *next;   // pointer to the next record
    record_ *prev;   // pointer to the previous record
} record;

Or perhaps stuff all the record objects into a real linked list ADT.


Bitwise comparison of any structure What I tried to do is compare arbitrary structures independent of the == operator implementation.

template<typename T>
bool bitWiseCompare(T a, T b)
{
  size_t size = sizeof(T) / sizeof(char);
  char* array_a = (char*)(&a);
  char* array_b = (char*)(&b);
  for(size_t i = 0; i<size; i++)
  {
    if(array_a[i] != array_b[i])
    {
      return false;
    }
  }
  return true;
}

This comparison can fail if there are any padding bytes within T, as those extra bytes are not initialized to any particular value.

Another source of failure can come from inheritance.  If one of the objects is part of a larger class (T is being used as a base class) with virtual functions, the vtable pointers can be different if a and b are part of different class hierarchies.  Virtual inheritance of base classes of T can also cause problems.

sizeof(char) is always 1, so there is no need to divide by it.

Since you're only reading from your parameters, you should take them by const &T to avoid making unnecessary copies.


Image uploader and manager I have recently started using Ruby and would like to hear your take on the following piece of code.

class Generator

  def initialize
    @context = nil
  end

  def start(params)
    @context = Context.new params
    image = create_image
    if cache_update_request?
      upload image
    end
    return image
  end

  def create_image
    composer = Composer.new @context
    execution_context = ExecutionContext.new(@context, composer)
    execution_context.render
  end

  def upload(image)
    uploader = CompositorCommons::CacheUpdater.new
    filename = @context.parameters[:cache]
    location = 'dual/' + filname
    type = 'image/jpeg'
    uploader.upload(image,location,type)
  end

  def cache_update_request?
    @context.parameters.has_key?(:cache)
  end

end

Your code is nice overall, but here are my two cents :

initializing instance variables before using them is not mandatory, so initialize as you implemented it is useless. A call to @context if it is not initialized will return nil.
Since uninitialized instance variables return nil, you may have have problem when doing things like @context.parameters[:cache]. As such, i would advise to initialize them... in initialize :

class Generator
  def initialize( params )
    @context = Context.new( params )
  end
end

This also leaves you the latitude to inspect the params and throw an ArgumentError if some usefull params are not present.

I think your object has mixed responsibilities. What does it represent ? It is not clear at first glance. Is it an uploader, an image ? It looks like a Service to me (the process is stateless), or maybe a "context" in DCI slang. Maybe your structure could more accurately represent this :

module ImageGeneratorService

  def self.start( params )
    cache = params.delete( :cache )
    image = ImageFactory.create_from_params( params )
    CacheService.cache_image( image, cache ) if cache
    image
  end

end

... ok, it's a bit too much, but i think you get the spirit. YMMV


Number pattern problem using Java Got this question in a test.

  Display the following number pattern using Java.

  User inputs number of rows. For example 3

1112
3222
3334

  For 5

111112
322222
333334
544444
555556

  and so on.

This is the solution I came up with:

   public static void NumberPattern(int n) {

    int myarray[][] = new int[n][n + 1];
    int i;
    int j;
    for (i = 0; i < 1; i++) //first row
    {
        for (j = 0; j < n; j++)
        {
            myarray[i][j] = i + 1;
        }
        myarray[i][n] = i + 2;
    }
    for (i = 1; i < myarray.length; i++)
       {
        if (i % 2 != 0)    //odd rows
           {
            j = 1;
            myarray[i][0] = i + 2;
            while (j <= n)
             {
                myarray[i][j] = i + 1;
                j++;
            }
            j = 1;
        }
        if (i % 2 == 0)   //even rows
        {
            j = 0;
            while (j < n) {
                myarray[i][j] = i + 1;
                j++;
            }
            myarray[i][n] = i + 2;
            j = 0;
        }
    }
    printArray(myarray);

}
 public static void printArray(int n[][])
  {
    System.out.println("The Pattern is:");

    for (int[] array : n) {
        for (int v : array) {
            System.out.print("  " + v);
        }
        System.out.println();
    }
  }

Is there a better way of solving this problem with only one for loop?

Since all you've been asked is to display output, a single simple nesting of two for loops without arrays should work just fine. Note that in all cases, you know in advance prior to performing a loop how many times the loop will iterate, and in this situation a for loop is cleaner than a while loop (though of course both would work):

// note that method namess should begin with a lower-case letter
public static void numberPattern(int rows) {
  for (int row = 0; row < rows; row++) {
     for (int col = 0; col < rows + 1; col++) {
        int number = 0;
        if (row % 2 == 0) {
           number = col < rows ? row + 1 : row + 2;
        } else {
           number = col == 0 ? row + 2 : row + 1;
        }
        System.out.print(number);
     }
     System.out.println();
  }
}

If you need to store the numbers to be used elsewhere, then yes, put them into an array.


Translating binary data from a file into a very large base-ten integer I'm making an experimental Java algorithm that translates binary data from a file into a (very large) base ten integer. I am using BigInteger since this number may have millions of digits. I am trying to translate this number into a sum of powers of 2.

I tried this code, which theoretically works, but is sluggishly slow. Using this code for a 200000 digit number takes about 5 minutes to complete with my i5 @4.5GHz. The execution time for a number with over a billion digits could take a day or more.

How could I optimize this code so that the execution time is significantly lowered?

byte[] bytes = Files.readAllBytes(outFile.toPath());
    StringBuilder sb = new StringBuilder();
    for (byte b : bytes) {
        sb.append(String.format("%02X", b));
    }
    BigInteger dataInt = new BigInteger(sb.toString(), 16);
    BigInteger remainder = dataInt;
    double dataLength = dataInt.bitCount();
    while (dataLength > BASE.bitCount()) {
        int increment = (int) Math.round(Math.pow(dataLength, 1 / BASE.doubleValue()));
        BigInteger base = BASE;
        int exp = 2;
        while (base.compareTo(remainder) < 0) {
            base = BASE.pow(exp);
            exp += increment;
        }
        while (base.compareTo(remainder) > 0) {
            base = BASE.pow(exp);
            exp--;
        }
        remainder = remainder.subtract(base);
        dataLength = remainder.bitCount();
    }

In this case, the constant BASE is 2.

byte[] bytes = Files.readAllBytes(outFile.toPath());

Reserving the fully needed space ensures not using extra space, and no reallocations.

    StringBuilder sb = new StringBuilder(bytes.length * 2);
    for (byte b : bytes) {
        sb.append(String.format("%02X", b));
    }
    BigInteger dataInt = new BigInteger(sb.toString(), 16);

Then however it would be faster to do:

    BigInteger dataInt = new BigInteger(1, bytes);

with the caveat, that this is a big endian interpretation. Reverting the bytes would be fast too however.

The actual algorithm unfortunately uses floating point, but that is your choice.
You could look at the sources of their implementation of toString(radix).

Also BigInteger has some interesting methods, like modPow or remainder.

Whether or not the loops can be optimized, pow multiplied by base, or the loop entirely eliminated. that is something interesting I leave upto you.


Waiting for bytes to be ready What this should do is wait for dataBlockSize bytes to be ready and then continue, but wait for 10ms if they are not ready yet to not block the CPU, but not the first time as I do not want to wait once even if the bytes are ready.

Is there a better way of doing this?

while (!cancellationToken.IsCancellationRequested)
{
    var ftdiStatus = ftdiDevice.GetRxBytesWaiting(ref rxBytes);
    if (ftdiStatus != FTDI.FT_STATUS.FT_OK)
        return null;

    if (rxBytes >= dataBlockSize)
        break;

    Thread.Sleep(10);
}

I think it is just a matter of taste. Here is one approach:

do
{
    var ftdiStatus = FtdiDevice.FtdiDevice.Instance.GetRxBytesWaiting(ref rxBytes);

    if (ftdiStatus != FTDI.FT_STATUS.FT_OK)
        return null;

    if(rxBytes >= dataBlockSize)
        break;

    Thread.Sleep(10);
}
while (true); // Perhaps a check for timeout?


Solution to Project Euler Problem #37 in C++: Truncatable Primes I was recently looking at some of my old Project Euler solutions, and saw that my implementation for Problem 37 could be improved drastically.
The problem reads as follows:

The number 3797 has an interesting property. Being prime itself, it is possible to continuously remove digits from left to right, and remain prime at each stage: 3797, 797, 97, and 7. Similarly we can work from right to left: 3797, 379, 37, and 3.
Find the sum of the only eleven primes that are both truncatable from left to right and right to left.
NOTE: 2, 3, 5, and 7 are not considered to be truncatable primes.

Here is my C++ solution to the problem:
#include <vector>
#include <iostream>
#include <cmath>
#include <algorithm>
#include <cstdlib>
#include <string>

std::vector<long long int> primesUpto(long long int limit) // Function that implements the Sieve of Eratosthenes
{
    std::vector<bool> primesBoolArray(limit, true);
    std::vector <long long int> result;

    primesBoolArray[0] = primesBoolArray[1] = false;
    long long int sqrtLimit = std::sqrt(limit) + 1;

    for (size_t i = 0; i < sqrtLimit; ++i)
    {
        if (primesBoolArray[i])
        {
            for (size_t j = (2 * i); j < limit; j += i)
            {
                primesBoolArray[j] = false;
            }
        }
    }
    for (size_t i = 0; i < primesBoolArray.size(); ++i)
    {
        if (primesBoolArray[i])
        {
            result.push_back(i);
        }
    }
    return result;
}

bool isTruncPrime(long long int number, const std::vector<long long int>& primeList)
{
    std::string numberString = std::to_string(number);
    for (int i = 1; i < numberString.size(); ++i)
    {
        std::string truncLeft = numberString.substr(0, i); // The truncated prime from the left
        std::string truncRight = numberString.substr(i, numberString.size() - 1); // The truncated prime from the right
        if (!
            (
                std::binary_search(primeList.begin(), primeList.end(), std::atol(truncLeft.c_str())) &&
                std::binary_search(primeList.begin(), primeList.end(), std::atol(truncRight.c_str()))
            )   // If either of the two truncated sides are not prime
        )
        {
            return false;
        }
    }
    return true; // All truncated parts are prime, so the number is a truncatable prime
}

int main()
{
    const std::vector<long long int> primesUptoMillion = primesUpto(1'000'000LL); // Represents all the primes up to 1 million

    int numberTruncatablePrimes = 0;
    long long int currentNumber = 9; // 2, 3, 5, and 7 are not included in the search for truncatable primes
    long long int truncatablePrimeSum = 0;

    while (numberTruncatablePrimes != 11)
    {
        if (
            std::binary_search(primesUptoMillion.begin(), primesUptoMillion.end(), currentNumber) && // If the number itself is prime
            isTruncPrime(currentNumber, primesUptoMillion) // If the number is also a truncatable prime
        )
        {
            ++numberTruncatablePrimes; // Increase amount of truncatable primes
            truncatablePrimeSum += currentNumber; // Add the number's sum
        }
        currentNumber += 2; // Only odd numbers can be prime other than 2, so no need to look at every number
    }
    std::cout << truncatablePrimeSum << "\n";
}

Here is how I run the code:
g++ Problem037.cpp -std=c++14 -O2

Here is code for execution, timing, and output:
$ time ./a.out
748317

real    0m0.042s
user    0m0.040s
sys     0m0.004s

Here are the things I want from this Code Review:

The function isTruncPrime() does a lot of string manipulation to verify its parameter. Is there a way to improve the algorithm?
While the code does run pretty quickly, is there any code that slows down the rest significantly that I could improve on?
Are there any better or more idiomatic ways to format/structure my program so it follows C++ style?

Warnings

It is usually a good idea to compile your code with all warnings enabled to see if anything is wrong. In your case, nothing really worrying but we may as well fix it.

Press ENTER or type command to continue
euler37.cpp: In function ‘std::vector<long long int> primesUpto(long long int)’:
euler37.cpp:16:26: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
     for (size_t i = 0; i < sqrtLimit; ++i)
                          ^
euler37.cpp:20:40: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
             for (size_t j = (2 * i); j < limit; j += i)
                                        ^
euler37.cpp: In function ‘bool isTruncPrime(long long int, const std::vector<long long int>&)’:
euler37.cpp:39:23: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
     for (int i = 1; i < numberString.size(); ++i)

You just need to use unsigned integers.

Algorithm

You could use a different approach to solve this problem. Instead of trying to find an efficient way to check if a number is a truncatable prime, you could try to find a way to generate them.
You could use the fact that the all right-truncable-primes of size n can be built easily from the list of right-truncable-primes of size n-1. The same logic applies to left-truncable-primes.

Here is a code example based on Toby Speight's code:

template<std::size_t N>
constexpr bool isLeftTruncPrime(std::size_t n, const std::array<bool,N>& primes)
{
    for (std::size_t x = 10;  x < n;  x*= 10)
        if (!primes[n%x]) return false;
    return true;
}

template<std::size_t N>
constexpr bool isRightTruncPrime(std::size_t n, const std::array<bool,N>& primes)
{
    for (;  n;  n/=10)
        if (!primes[n]) return false;
    return true;
}

template<std::size_t N>
constexpr bool isTruncPrime(std::size_t n, const std::array<bool,N>& primes)
{
    return isRightTruncPrime(n, primes) && isLeftTruncPrime(n, primes);
}

#include <iostream>
int main()
{
    const auto limit = 1'000'000;
    static const auto primes = primesUpto<limit>();
    auto expected_sum = 748317ull;

    if (1)
    {
        auto sum = 0ull;
        // Start at 11, because single-digit numbers aren't truncatable
        // Advance by 2, because even numbers are non-prime (except 2, but 2<11)
        for (std::size_t n = 11;  n < primes.size();  n += 2) {
            if (isTruncPrime(n, primes))
                sum += n;
        }

        std::cout << sum << "\n";
        std::cout << (sum == expected_sum) << "\n";
    }

    if (0)
    {
        auto sum = 0ull;
        std::vector<int> truncprimes;
        truncprimes.push_back(2);
        truncprimes.push_back(3);
        truncprimes.push_back(5);
        truncprimes.push_back(7);

        while (!truncprimes.empty())
        {
            std::vector<int> truncprimes2;
            for (auto tp: truncprimes)
            {
                auto tens = tp*10;
                for (size_t d = 1; d < 10; d+=2)
                {
                    auto n = tens + d;
                    if (n < limit && primes[n])
                    {
                        truncprimes2.push_back(n);
                        if (isLeftTruncPrime(n, primes) /*&& n > 10 */)
                            sum += n;
                    }
                }
            }
            truncprimes = truncprimes2;
        }

        std::cout << sum << "\n";
        std::cout << (sum == expected_sum) << "\n";
    }
}

Then, because of the small amount of prime check we perform, pre-computing a sieve is not worth the pain. Thus, you don't even need to assume a maximum value. You don't even need to assume there will be 11 numbers found.

The new code is way faster than the original code:

bool isPrime(std::size_t num)
{
    if(num <= 1) return false; // <- 1 is not prime
    for (std::size_t i = 2; i * i <= num; i++)
        if (num % i == 0) return false;
    return true;
}

bool isLeftTruncPrime(std::size_t n)
{
    for (std::size_t x = 10; x < n; x*= 10)
        if (!isPrime(n%x)) return false;
    return true;
}

#include <iostream>
int main()
{
    auto expected_sum = 748317ull;
    if (1)
    {
        auto sum = 0ull;
        std::vector<int> truncprimes;
        truncprimes.push_back(2);
        truncprimes.push_back(3);
        truncprimes.push_back(5);
        truncprimes.push_back(7);

        while (!truncprimes.empty())
        {
            std::cout << truncprimes.size() << "\n";
            std::vector<int> truncprimes2;
            for (auto tp: truncprimes)
            {
                auto tens = tp*10;
                for (size_t d = 1; d < 10; d+=2)
                {
                    auto n = tens + d;
                    if (isPrime(n))
                    {
                        truncprimes2.push_back(n);
                        if (isLeftTruncPrime(n))
                            sum += n;
                    }
                }
            }
            truncprimes = truncprimes2;
        }

        std::cout << sum << "\n";
        std::cout << (sum == expected_sum) << "\n";
    }
}

Conclusion

I really like this problem and this solution because it shows that going straight into the Eratosthene sieve is not always the perfect solution.


Most efficient implementation of Djisktra's shortest path using vectors and pairs in C++ stl If std::vector<vector<pair<int,int> > > v(n) represents the adjacency list of the graph with pair<int,int> is the {vertex,weight} pair, I tried to implement the algorithm the following way:

while(true)
        {
            long long yo = LLONG_MAX;
            int ind=-1;
            for(int i=0;i<n;++i)
            {
                if(ans[i]<yo && !v[i].empty())
                {
                    ind=i;
                    yo=ans[i];
                }
            }
            if(ind==-1)
                break;
            for(int i=0;i<v[ind].size();++i)
            {
                if(ans[v[ind][i].first] > ans[ind]+v[ind][i].second)
                    ans[v[ind][i].first] = ans[ind]+v[ind][i].second;
                v[ind].erase(v[ind].begin()+i);
            }
        }

Where ans[i] stores the shortest paths which is initialised as {LLONG_MAX,...0,...LLONG_MAX}, 0 being the source. Since this is the first time I tried implementing it, is there a better way to implement using the vectors/list in stl (in terms of time/space complexity maybe)?

for(int i=0;i<v[ind].size();++i) {
    ...
    v[ind].erase(v[ind].begin()+i);
}

It looks like a bug. For instance, when you erase the 0-th element all other elements get shifted, so the new element at position 0 will never be deleted. You can just iterator over the vector and clear it after the loop.

I don't see the point in removing the graph edges after the vertex is processed. I'd suggest to create a separate vector (used or visited) to keep track of processed vertices and leave the input graph unchanged.

If the graph is sparse, you can improve the time complexity from O(|V|^2) to O(|E| log |V|) or even O(|E| + |V| log |V|) using an appropriate data structure (like a heap).

I'd also suggest to create a separate class for an edge instead of using std::pair<int, int>. It's hard to confuse edge.weight with edge.destination_vertex. It's easy to confuse the first and the second elements of the pair (it's not easy to keep the meaning of the first and the second elements of the pair in one's head).

You can make the code more readable by naming your variables in a more meaningful way (for instance, what is yo? It's something like min_distance, isn't it?).

When you need to iterate over a container and you don't need elements' indices, you can use a range-based for loop. For instance, this:

for(int i=0;i<v[ind].size();++i)
{
    if(ans[v[ind][i].first] > ans[ind]+v[ind][i].second)
        ans[v[ind][i].first] = ans[ind]+v[ind][i].second;
    v[ind].erase(v[ind].begin()+i);
}

could be

for (auto edge: v[ind]) { // or const auto& edge for heavier objects
// do something with the edge
}

You can also improve the readability of your code by creating smaller functions with a meaningful name that do one focused thing (like finding the closest vertex and so on) and call them from the function that implements Dijkstra's algorithm itself. Here's how I'd do it:

// Instances of this struct represent graph edges
struct Edge {
    int weight;
    int destination_vertex;

    Edge(int _weight, int _destination_vertex):
        weight(_weight), destination_vertex(_destination_vertex) {}
};

// Finds the closest unvisited vertex
int get_closest_unvisited_vertex(const vector<long long>& dist,
        const vector<bool>& visited) {
    int closest_vertex = -1;
    for (size_t v = 0; v < dist.size(); v++) {
        if (!visited[v]
                && (closest_vertex == -1 || dist[v] < dist[closest_vertex])) {
            closest_vertex = v;
        }
    }
    return closest_vertex;
}

// Updates the distances to all neighbors of the given vertex
// The dist vector is passed by reference to avoid copying the data
void relax_vertex(int vertex, const vector<vector<Edge>>& graph,
        vector<long long>& dist) {
    for (auto edge : graph[vertex]) {
        dist[edge.destination_vertex] = min(dist[edge.destination_vertex],
                dist[vertex] + edge.weight);
    }
}

// Finds the distance from the source vertex to all other vertices in
// the graph
vector<long long> get_distance_from(const vector<vector<Edge>>& graph,
        int source) {
    vector<long long> dist(graph.size(), numeric_limits<long long>::max());
    dist[source] = 0;
    vector<bool> visited(graph.size());
    while (true) {
        int closest_vertex = get_closest_unvisited_vertex(dist, visited);
        if (closest_vertex == -1
                || dist[closest_vertex] == numeric_limits<long long>::max()) {
            break;
        }
        visited[closest_vertex] = true;
        relax_vertex(closest_vertex, graph, dist);
    }
    return dist;
}


Battleships “AI” function I would like some advice on how I could shorten my function from my battleships game (below). Essentially what it does is it checks if a coordinate is a hit and if it is, then if checks adjacent coords and shoots the one that would be a hit (it cheats, basically, which is fine).

I would like to know what I could do to make it more effective/shorter/more neat.

(If you improve it with techniques that I am not currently using, then please explain said technique. I want to use something that I can understand.)

def ai_shoots(y_coord, x_coord, all_buttons, player_1_board, ai_score):
    # print("yes")
    if ai_score == 20:
        popupwindow("The computer has won.")
    if player_1_board[y_coord][x_coord] == ': ':
        ai_score += 1
        player_1_board[y_coord][x_coord] = 'X '
        all_buttons[y_coord - 1][x_coord - 1].configure(text="X", fg="black", bg="red3")
        if player_1_board[y_coord - 1][x_coord] == ': ':
            ai_shoots(y_coord - 1, x_coord, all_buttons, player_1_board, ai_score)
        elif player_1_board[y_coord + 1][x_coord] == ': ':
            ai_shoots(y_coord + 1, x_coord, all_buttons, player_1_board, ai_score)
        elif player_1_board[y_coord][x_coord - 1] == ': ':
            ai_shoots(y_coord, x_coord - 1, all_buttons, player_1_board, ai_score)
        elif player_1_board[y_coord][x_coord + 1] == ': ':
            ai_shoots(y_coord, x_coord + 1, all_buttons, player_1_board, ai_score)
        else:
            x = random.randint(1, 10)
            y = random.randint(1, 10)
            ai_shoots(y, x, all_buttons, player_1_board, ai_score)
    elif player_1_board[y_coord][x_coord] == 'X ' or player_1_board[y_coord][x_coord] == 'O ':
        x = random.randint(1, 10)
        y = random.randint(1, 10)
        ai_shoots(y, x, all_buttons, player_1_board, ai_score)
    else:
        player_1_board[y_coord][x_coord] = 'O '
        all_buttons[y_coord - 1][x_coord - 1].configure(text="O", fg="white")

logic/presentation

You are mixing presentation and business logic. Both when signalling the AI won, as with the board.
Do you want to look through your whole code if you decide that "_" s a better representation for clear sea than "O", or you want it in another colour than white?

Tile

As far as I remember correctly, a tile on a battleships board has a few states: (clear sea, vessel, shot but no hit, hit vessel), where clear sea and vessel have the same representation to the opponent. So instead of using a string to represent this state, better would be to use an enum

from enum import Enum

class Tile(Enum):
    CLEAR = "CLEAR"
    VESSEL = "VESSEL"
    SHOT = "SHOT"
    HIT = "HIT"

Board

I assume your board at this moment is just a list of lists of strings. It is worth it to make a slightly smarter board

class Board:
    def __init__(self, width=10, height=10):
        self.width = width
        self.height = height
        self._board = [[Tile.CLEAR] * width for _ in range(height)]

is used to create an empty board

By overloading __getitem__ and __setitem__, you can make accessing the board a lot more intuitive:

def _exists(self, x, y):
    return 0 < x <= self.height and 0 < y <= self.width

def __getitem__(self, coord):
    x, y = coord
    if not self._exists(x, y):
        raise IndexError
    return self._board[x-1][y-1]

def __setitem__(self, coord, state):
    x, y = coord
    if not self._exists(x, y):
        raise IndexError
    self._board[x-1][y-1] = state

now you can do stuff like

b = Board()
b[2, 3]

Tile.Clear

b[2, 3] = Tile.Vessel
b[2, 3]

Tile.Vessel

This way, you don't bother the user directly with the 0-indexed list of lists in a 1-indexed environment. You can even work with row names A to J if you want with only a slight adaptation to those 2 methods.

Then you can add a ship like this:

def add_ship(self, length, x, y, orientation='horizontal'):
    directions = {
        'horizontal': (1, 0),
        'vertical': (0, 1),
    }
    dx, dy = directions[orientation]
    coordinates = [
        (x + i * dx, y + i *dy)
        for i in range(length)
    ]
    try:
        if any(self[x, y] != Tile.CLEAR for x, y in coordinates):
            raise ValueError("No room for this ship")
    except IndexError: # over the edge
        raise ValueError("No room for this ship")
    for x, y in coordinates:
        self._board[x, y] = Tile.VESSEL

In your implementation of the AI, after a hit, you have the AI check all the neighbours. This can be simplified by implementing a neighbours function on the Board

def already_shot(self, x, y):
    return self[x, y] in {Tile.SHOT, Tile.HIT}

def neighbours(self, x, y):
    coordinates = ((x-1, y), (x+1, y), (x, y-1), (x, y+1))
    for x, y in coordinates:
        try:
            yield (x, y), self.already_shot(x, y)
        except IndexError:
            pass

Same goes for the actual shooting itself:

def shoot(self, x, y):
    if self.already_shot(x, y):
        raise ValueError("Already targeted this square")

    if self[x, y] == Tile.VESSEL:
        self[x, y] = Tile.HIT
        return True
    self[x, y] = Tile.SHOT
    return False

You will need to implement a method to check whether all ships are sunk to determine whether someone has won. This method belongs to the Board, instead of on the AI method

Testing

using a class like this to keep track of the state of the board allows you to test parts of your program piece-meal. You can write a test suite so test the placing of the ships, the shooting, etc.

Turns

Where do turns show up in this? As far as I understand this, the AI keeps shooting as long as it can hit. I would expect the AI to be implemented as a class, which remembers what it has already shot etcetera, and which gets queried for a coordinate to aim at by the Game

Random shooting

If the minimum length of a ship is 2, you only need to target half of the field in while randomly looking for a ship, so only those where (x + y) % 2 is either 1 or 0, depending on where you start

AI class

This is a first shot at it. The _last_hit can use some improvement, and using a collections.deque or even a set here seems more logical than using a single value. But it shows how different methods can be used for defined beahviour

class AI:
    def __init__(self, opponent_board):
        self._board = opponent_board
        self._last_hit = None

    def random_shot(self):
        while True:
            x = random.randint(1, self._board.height)
            y = random.randint(1, self._board.width)
            if (x + y) % 2 == 0:
                continue
            try:
                hit = self._board.shoot(x, y)
                self.mark_hit(x, y, hit)
                return hit
            except ValueError:
                continue

    def mark_hit(self, x, y, hit):
        if hit:
            self._last_hit = x, y
        else:
            self._last_hit = None

    def next_turn(self):
        if self._last_hit is None:
            return self.random_shot()
        x, y = self._last_hit
        if self._board[x, y] == Tile.HIT:
            for x_, y_ in self._board.neighbours(x, y):
                try:
                    hit = self._board.shoot(x, y)
                    self.mark_hit(x, y, hit)
                    return hit
                except ValueError:
                    pass
            self._last_hit = None

        # No adjacent tiles to aim at if this got through here
        return self.random_shot()


Make this Ruby array pivot more efficient? A puzzle I was given:

  Description:

  Write a method that returns the "pivot" index of a list of integers.
  We define the pivot index as the index where the sum of the numbers on
  the left is equal to the sum of the numbers on the right. Given [1, 4,
  6, 3, 2], the method should return 2, since the sum of the numbers to
  the left of index 2 is equal to the sum of numbers to the right of
  index 2 (1 + 4 = 3 + 2). If no such index exists, it should return -1.
  If there are multiple pivots, you can return the left-most pivot.
  You can write the method in any language.

  Make sure that the method:

  runs successfully
  handles all edge cases
  is as efficient as you can make it!

Apparently my solution is not efficient enough:

def pivot(arr)
  results = []
  arr.each_with_index do |n,index|
    last      = arr.size - 1
    sum_left  = arr[0..index].inject(:+)
    sum_right = arr[index..last].inject(:+)
    results << index if sum_left == sum_right
  end
  results.any? ? results.first : -1
end

Ruby allows -1 as an index that means last, so you don't have to calculate it at all.

Calculating the whole left_sum every time is repeating work since it is always the previous left_sum + arr[index-1] (except for when index = 0). Similarly the right_sum is always the previous right_sum - arr[index].

You don't have to gather all results, so you can terminate early on finding the leftmost solution, or as soon as sum_left > sum_right (assuming there are no negative numbers in arr?) you know there is no solution, so can return -1.

For example (untested)

def find_pivot(arr)
  sum_left = -arr[-1]
  sum_right = arr.inject(:+)
  arr.each_index do |i|
    sum_left  += arr[i-1]
    sum_right -= arr[i]
    if sum_left == sum_right
      return i
    elsif sum_right < sum_left
      # assuming there are no negative numbers we already know there's no solution
      return -1
    end
  end
  return -1 # in case we somehow reach the end without a solution or early termination
end

Initialising sum_left to -arr[-1] is a trick to save on having to add an if statement to detect and handle the first iteration of the loop differently, since it cancels out the effect of sum_left += arr[0-1] which would make sum_left jump to the value of the last value in the array.


Game of Life with Scala and Functional Programming I have been doing OOP and imperative programming my whole career but I wanted to dabble in Scala along with functional programming. Rather than writing Java like code in Scala I wanted to try a more functional approach. I decided to try a simple Kata with Scala, Game of Life. The following is what I came up with. I admit I am very much stuck in OOP/imperative mindset. I am not sure if this is the best approach for Scala, or functional programming. I avoided functions that mutate objects, and favor returning a new copy. I was unsure what the best approach is with functional programming to ensure objects can't be created in an invalid state. Is there a way I can make my code more aligned with idiomatic Scala and functional programming.

package io.jkratz.katas.gameoflife

import scala.util.Random

case class Board(grid: Array[Array[Int]]) {

  require(grid != null, "grid cannot be null")
  require(!isJagged(grid), "grid cannot be jagged")
  require(isValid(grid), "grid contains invalid values, 0 and 1 are the only valid values")

  val rows: Int = {
    grid.length
  }

  val columns: Int = {
    grid(0).length
  }

  def evolve(): Board = {

    val newGrid = Array.ofDim[Int](rows, columns)
    for (i <- grid.indices) {
      for (j <- grid(0).indices) {
        newGrid(i)(j) = getNextCellState(i,j)
      }
    }

    Board(newGrid)
  }

  private def getNextCellState(i:Int, j: Int): Int = {
    var liveCount = 0
    val cellValue = grid(i)(j)
    for (x <- -1 to 1; y <- -1 to 1) {
      if (i + x < 0 || i + x > (this.rows - 1) || y + j < 0 || y + j > (this.columns - 1)) {
        // do nothing, out of bounds
      } else {
        liveCount += grid(i + x)(j + y)
      }
    }
    liveCount -= cellValue

    if (cellValue.equals(Board.CELL_ALIVE) && (liveCount < 2 || liveCount > 3)) {
      Board.CELL_DEAD
    } else if (cellValue.equals(Board.CELL_DEAD) && liveCount == 3) {
      Board.CELL_ALIVE
    } else {
      cellValue
    }
  }

  private def isJagged(grid: Array[Array[Int]]): Boolean = {
    var valid = true
    val size = grid(0).length
    grid.foreach(row => if (row.length.equals(size)) valid = false)
    valid
  }

  private def isValid(grid: Array[Array[Int]]): Boolean = {
    var valid = true
    for (i <- grid.indices; j <- grid(0).indices) {
      val x = grid(i)(j)
      if (x != 0 && x != 1) {
        valid = false
      }
    }
    valid
  }
}

object Board {

  val CELL_DEAD = 0
  val CELL_ALIVE = 1
  val DEFAULT_ROWS = 10
  val DEFAULT_COLUMNS = 10

  def random(rows: Int = DEFAULT_ROWS, columns: Int = DEFAULT_COLUMNS): Board = {
    val grid = Array.ofDim[Int](rows, columns)
    for (i <- grid.indices) {
      for (j <- grid(0).indices) {
        grid(i)(j) = Random.nextInt(2)
      }
    }
    Board(grid=grid)
  }

  def prettyPrint(board: Board): Unit = {
    val grid = board.grid
    for (i <- grid.indices) {
      for (j <- grid(0).indices) {
        if (grid(i)(j) == 0) print(" - ") else print(" * ")
      }
      println()
    }
  }
}

And my main entry point.

package io.jkratz.katas.gameoflife

object Life {

  def main(args: Array[String]): Unit = {

    val state0 = Board.random(5,5)
    println("State 0")
    Board.prettyPrint(state0)

    val state1 = state0.evolve()
    println("State 1")
    Board.prettyPrint(state1)
  }
}

You're doing a lot of indexing, which is efficient in an Array, but indicates that you're thinking in small steps. As you get to know the Scala Standard Library you start thinking in larger chunks because it offers many ways to process data collections all at once.

I'll start at the bottom and work my way up.

*prettyPrint() - Testing for value 0 means that the prettyPrint() method knows about the underlying representation. If you test for CELL_DEAD and/or CELL_ALIVE then prettyPrint() should still work even if the grid implementation changes.

Here I chose to turn each row into a String and then println() it.

def prettyPrint(board: Board): Unit =
  board.grid
       .map(_.map(c => if (c == CELL_DEAD) " - " else " * ").mkString)
       .foreach(println)

*random() - Most Scala collections offer many different "builder" methods on the companion object. Here I use fill() to populate a 2-dimensional Array.

def random(rows:Int = DEFAULT_ROWS, columns:Int = DEFAULT_COLUMNS): Board =
  Board(Array.fill(rows,columns)(Random.nextInt(2)))

*isValid() - The Standard Library doesn't offer many collection methods with early termination, but forall() is one of them. It stops after the first false encountered.

Here I use -2 as a bit-mask to test the value of all bits except for the lowest.

private def isValid(grid: Array[Array[Int]]): Boolean =
  grid.forall(_.forall(n => (n & -2)==0))

*isJagged() - The exists() method is the compliment of forall(). It stops after the first true encountered.

private def isJagged(grid: Array[Array[Int]]): Boolean =
  grid.exists(_.length != grid.head.length)

*liveCount - Idiomatic Scala avoids mutable variables. In order to calculate the value of liveCount once, without any post-evaluation adjustments, we'll want a way to get all valid neighbor-cell indexes, also without any post-evaluation adjustments.

val liveCount = (for {
  x <- (0 max i-1) to (i+1 min rows-1)
  y <- (0 max j-1) to (j+1 min columns-1)
} yield grid(x)(y)).sum - cellValue

*evolve() - tabulate() is another one of those "builder" methods that appears to do everything you need, in this situation, all at once. In this case, because we're building a 2-D Array, tabulate() passes two arguments, the row index and the column index, to its lambda argument. And because the getNextCellState() method takes those same two arguments in the same order, they don't need to be explicitly specified.

def evolve(): Board = Board(Array.tabulate(rows,columns)(getNextCellState))

It's worth noting that you test if grid is null but you don't test to see if it's empty. Board.random(0,0) will throw a run-time error.


Angular console like window - first directive I've asked question on SO how to rewrite jQuery code to Angular. Because I got no answers I've tried to create something by my own.
This is my version of code:

(function() {
  'use strict';

  angular
    .module('misiu.directives', [])
    .directive('console', ['$rootScope', function($rootScope) {
      return {
        restrict: 'E',
        template: [
          '
',
          '
',
          '{{event.data}}1" class="count">{{event.count}}',
          '
',
          '
'
        ].join(''),
        link: function($scope, element) {

          $scope.events = [];

          $rootScope.$on('Log', function(event, data) {

            if ($scope.events[0] !== undefined && $scope.events[0].type === "log" && $scope.events[0].data === "Log :: " + data) {
              $scope.events[0].count++;
            } else {
              $scope.events.splice(0, 0, {
                type: 'log',
                data: "Log :: " + data,
                count: 1
              });
            }

          });

          $rootScope.$on('Error', function(event, data) {

            if ($scope.events[0] !== undefined && $scope.events[0].type === "error" && $scope.events[0].data === "Error :: " + data) {
              $scope.events[0].count++;
            } else {
              $scope.events.splice(0, 0, {
                type: 'error',
                data: "Error :: " + data,
                count: 1
              });
            }

          });
        }
      };
    }])
})();

and Plunker showing demo (move mouse over console div to add entries)

Because this is my first directive I'd like to ask for code review.

I'm adding new items to my console using one of those lines:

$rootScope.$emit("Log", "Something");
$rootScope.$emit("Error", "Terrible error");

I must add $rootScope as dependency to every controller if I want to use my directive there. Maybe factory would be better? This way I could for example add a way to remove everything except last 20 entries.

I'd like my directive to be optimized for speed. I've tested it for a while, but I'm not sure I did everything properly.

I need only single way binding and the thing that can change it only top most element (counter can increase), rest of element don't need binding. Maybe this can be also optimized.

Any other comments on what is wrong are welcome.

EDIT
I've created another code review request that includes same control, but using provider.

template: [
      '
',
      '
',
      '{{event.data}}1" class="count">{{event.count}}',
      '
',
      '
'
    ].join(''),

If you have a build step with a transpiler, or don't care about old browsers, consider using template strings instead. Template strings can multi-line, making it easy to build markup.

$rootScope.$on('Log', function(event, data) {

$rootScope is great for data relay. However, when an app grows, you will be caught in a tangle of events. You don't know who fired them, what they'll execute, their payload (unless you document them, which rarely happens). Plus, $rootScope is a global and we all know what happens to globals when they get polluted. Suggesting you use a service/factory instead. That way, it isn't global plus you have some form of control.

Also, instead of separate events, why not a single event. It seems like your logic already sorts itself out by the type property.

$scope.events[0] !== undefined

Looks like you're checking if the array contains something. It would be best expressed if you checked the array length property instead.

$scope.events.splice(0, 0, {
  type: 'log',
  data: "Log :: " + data,
  count: 1
});

There's array.unshift. It's like array.push but puts the new value at the start of the array.


Merge Sort implementation with generators and recursion Just beating the dead merge-sort horse, this time with generators and recursion!

What do you think?

p.s. Not entirely sure why stack overflow needs me to post more details on a code review. I'm implementing something straight from an undergrad CS class but because it does, here Is the video that inspired me https://www.youtube.com/watch?v=TzeBrDU-JaY

k=[1,2,1,3,4,2]
def merge_sort(lis):
    """this performs merge sort assuming
none of the values in lis are greater than 10**100"""
    if len(lis)<=1:
        return lis
    else:
        over=int(len(lis)/2)
        RL=lis_generator(merge_sort(lis[over:]))
        LL=lis_generator(merge_sort(lis[:over]))
        new_l=list()
        cRL=next(RL)
        cLL=next(LL)
        while len(new_l)<len(lis):
            if cRL<=cLL:
                new_l.append(cRL)
                cRL=next(RL)
            else:
                new_l.append(cLL)
                cLL=next(LL)

        return new_l

def lis_generator(lis):
    """enumerate --> index, value"""
    for i in lis:
        yield i
    yield 10**100

You don't actually get benefits from using generators in this particular implementation since you are still slicing actual lists having the slices created in memory. And, you also collect the results into a new_l list being created in memory and returned all at once.

You should use iterative islice() like demonstrated in this example. Also, check out this generator-based merge sort implementation - note how the results are yielded from the "merge sort" function.

Some code style related notes:

since you return from the function when len(lis) <= 1, you can omit the else: part and decrease the indentation level of the case when len(lis) is more than 1
improve your variable naming - lis is not a good way to workaround not naming a variable list, over should probably be middle, cRL and cLL should probably become left and right etc.
documentation strings should start with a capital letter and end with a dot, according to PEP8 style guide (reference)
watch for other PEP8 code style violations - specifically, watch for spaces around operators (reference)


Unit Testing Android Studio I am carrying out unit testing for my project but i am not sure i am doing it correctly. The example below calculates the difference between 2 time duration's.

public void diffTime2() throws Exception {

        //Calculsates the difference
        String ExpectedResult = "1:0:0";
        String str_time1 = "04 13, 2018 09:00:00";
        String str_time2 = "04 13, 2018 08:00:00";
        DateFormat simpleDateFormat3 = new SimpleDateFormat("MM dd, yyyy HH:mm:ss");
        Date date1 = simpleDateFormat3.parse(str_time1);
        Date date2 = simpleDateFormat3.parse(str_time2);
        DateTime dt1 = new DateTime(date2);
        DateTime dt2 = new DateTime(date1);
        long hours = (Hours.hoursBetween(dt1, dt2).getHours() % 24);
        long mins = (Minutes.minutesBetween(dt1, dt2).getMinutes() % 60);
        long secs = (Seconds.secondsBetween(dt1, dt2).getSeconds() % 60);
        String result = (hours+":"+mins+":"+secs);
        //System.out.println(hours+":"+mins+":"+secs);

        assertEquals(ExpectedResult, result);
}

It successfully runs and seems to be OKbut i am not sure that i am completing the unit testing exactly how i should.

Any observation about this is greatly appreciated

Some notes:

This is not a unit test of your code. A unit test must at some point involve some part of your code, and it looks like this test uses only classes which exist in the Java SDK. The idea is not to check an idea for an implementation, but instead to detect bugs in your actual implementation.
By convention instance variables use camelCase in Java, so ExpectedResult should be expectedResult and str_time2 should be strTime2.
Maybe it's necessary, but I'm sceptical of the need for the conversion from Date to DateTime.


Check if a word is before another Let's consider a method like this:

public bool IsWordABeforeWordB(string Text, string WordA, string WordB)
{
   ...
}

By Word I mean a string composed of more than one character.

Test cases:

[TestMethod]
public void IsWordABeforeWordB_ShouldBeTrue()
{
    string Text = "CONSTRAINT [PK_Test] PRIMARY KEY CLUSTERED ([IDTest] ASC)";
    bool result = AtomicCheckMethods.IsWordABeforeWordB(Text, "CLUSTERED", "ASC");
    result.Should().BeTrue();
}

[TestMethod]
public void IsWordABeforeWordB_ShouldBeFalse()
{
    string Text = "CONSTRAINT [PK_Test] PRIMARY KEY CLUSTERED ([IDTest] ASC)";
    bool result = AtomicCheckMethods.IsWordABeforeWordB(Text, "PRIMARY KEY", "CONSTRAINT");
    result.Should().BeFalse();
}

[TestMethod]
public void IsWordABeforeWordB_IgnoreCase_ShouldBeTrue()
{
    string Text = "CONSTRAINT [PK_Test] PRIMARY KEY CLUSTERED ([IDTest] ASC)";
    bool result = AtomicCheckMethods.IsWordABeforeWordB(Text, "clustered", "asc");
    result.Should().BeTrue();
}

[TestMethod]
public void IsWordABeforeWordB_IgnoreCase_ShouldBeFalse()
{
    string Text = "CONSTRAINT [PK_Test] PRIMARY KEY CLUSTERED ([IDTest] ASC)";
    bool result = AtomicCheckMethods.IsWordABeforeWordB(Text, "primary key", "constraint");
    result.Should().BeFalse();
}

What I tried:

public static bool IsWordABeforeWordB(string Text, string WordA, string WordB)
{
    string stringBeforeWordB = Text.Substring(0, Text.IndexOf(WordB, StringComparison.InvariantCultureIgnoreCase));
    return stringBeforeWordB.IndexOf(WordA, StringComparison.InvariantCultureIgnoreCase) >= 0;
}

Can I do it better?

You're naming your parameters incorrectly. The naming guidelines for parameters specify camel casing.

Your method declaration should look like this:

public static bool IsWordABeforeWordB(string text, string wordA, string wordB)

Your current implementation also has a bug. You are assuming that the source text contains wordB - if it doesn't you'll get an ArgumentOutOfRangeException on the substring.

I'd suggest that performance probably doesn't matter here so you may as well just do it the long way:

public static bool IsWordABeforeWordB(string text, string wordA, string wordB)
{
    // Omitted checking text, wordA and wordB for null.
    var indexOfFirstWordA = text.IndexOf(wordA, StringComparison.OrdinalIgnoreCase);
    if (indexOfFirstWordA == -1)
    {
         // throw exception or return false.
    }
    var indexOfLastWordB = text.LastIndexOf(wordB, StringComparison.OrdinalIgnoreCase);
    if (indexOfLastWordB == -1)
    {
        // throw exception or return false.
    }

    // edited based on holroy's comment.
    // Otherwise we'd be wrong if wordB was a substring of wordA
    // e.g. wordA: hello, wordB: lo
    return indexOfFirstWordA + wordA.Length < indexOfLastWordB;

}

I've assumed that IsWordABeforeWordB("Sample text is sample", "text", "sample") should return true as "text" does appear before one of the "sample"s. If that's not the behaviour you were going for you can change the LastIndexOf to a simple IndexOf

The definition of a "word" is a bit loose here. You could do a Regular Expression match to get the indexes which would allow you to use the "\b" word anchor. I've just assumed (as your OP) that it's simply a sequence of characters.


Building complex sentences based on a data structure I really do not like how bad this looks. It is not readable and I am not sure if I need the stacked map's. There are a lot of iterations going on here (map, join, replace) and performance is really important, please help :(

Input:

const alternatives = [
  "flex",
  "float",
  [
    "background-size",
    "background-image"
  ]
]

Expected Output:

'Consider using 'flex', 'float' or 'background-size' with 'background-image' instead.'

Working Code:

const result = `Consider using ${alternatives
                 .map((alternative) => {
                   return Array.isArray(alternative)
                     ? alternative.map((a) => `'${a}'`).join(' with ')
                     : `'${alternative}'`;
                 })
                 .join(', ')
                 .replace(/, ([^,]*)$/, ' or $1')} instead.`

Remove the last item

You are not going to avoid the stepping over each item at least once. However the replace and be avoided by removing the last item.

One iteration

Use a for loop,  building the string as you step over each item. Before the loop pop the last item, then after the loop push it back to keep the array intact.  You can then add the last item after the conjunction.

I will use options rather than alternatives as in the bottom example it is more semantically fitting.

function toHumanReadable(options) {
    const toStr = opt => `"${Array.isArray(opt) ? `${opt[0]}" with "${opt[1]}` : opt}"`;
    const last = options.pop();
    var res = 'Consider using ';
    for (const opt of options) { res += toStr(opt) + ', ' }
    options.push(last);
    return res + `or ${toStr(last)} instead.`;
}
const alt1 = [ "flex", "float" , ["background-size", "background-image" ] ];
const readable = toHumanReadable(alt1);

or you can make options an argument array, and don't need to push the last item back onto the array. It does mean an extra iteration.

function toHumanReadable(...options) {
    const toStr = opt => `"${Array.isArray(opt) ? `${opt[0]}" with "${opt[1]}` : opt}"`;
    const last = options.pop();
    var res = 'Consider using ';
    for (const opt of options) { res += toStr(opt) + ', ' }
    return res + `or ${toStr(last)} instead.`;
}

const alt1 = [ "flex", "float" , ["background-size", "background-image" ] ];
const readable = toHumanReadable(...alt1);

Reusable

I am guessing this is just one of a many similar human readable strings. To make the function reusable you can add some arguments

The next version requires two iterations, but does not need the replace. This time the last item is popped from a copy of the options array and the conjunction is added via joining the joined options and the last option.

function toHumanReadable(options, open, close, conjunction) {
    options = options.map(opt => Array.isArray(opt) ? opt.join('" with "') : opt);
    const last = options.pop();
    return `${open} "${[options.join('", "'), last].join(`", ${conjunction} "`)}" ${close}`;
}

const alt1 = [ "flex", "float" , ["background-size", "background-image" ] ];
const readable = toHumanReadable(alt1, "Consider using", "instead.", "or");

Example usage

setTimeout(()=> { // just for snippet to put relevant code at top

  const alt1 = [ "flex", "float" , ["background-size", "background-image" ] ];
  const alt2 = [ "foo", ["bar","poo"], ["min-width", "max-height" ] ];
  log(toHumanReadable(alt1, sentenceTypes.anyOf));
  log(toHumanReadable(alt2, sentenceTypes.oneOf));
  log(toHumanReadable(alt1, sentenceTypes.allOf));
},0);

function toHumanReadable(options, {open, close, conjunction}) {
    options = options.map(opt => Array.isArray(opt) ? opt.join('" with "') : opt);
    const last = options.pop();
    return `${open} "${[options.join('", "'), last].join(`", ${conjunction} "`)}" ${close}`;
}

const sentenceTypes = {
   create(name, open, close, conjunction) { this[name] = {open, close, conjunction} }
};
sentenceTypes.create("anyOf", "Consider using", "instead.", "or");
sentenceTypes.create("oneOf", "Use any one of", "to stay consistent.", "or");
sentenceTypes.create("allOf", "All the following", "are required.", "and");
function log(t){txt.appendChild(Object.assign(document.createElement("div"),{textContent:t}))}
body {
font-family: arial;
font-size: small;
}


4×4 matrix multiplication This is a simple C++ code with a function mult to multiply matrices. This can easily be generalized for any \$n \times n\$ matrix by replacing 4 with any positive number greater than 1.

The multiplication is done by iterating over the rows, and iterating (nested in the rows iteration) over the columns. While inside the columns iteration, the multiplication is performed, which is a dot product (again using a new iteration).

The simple demo shows the multiplication of \$A \times A\$.

I wonder if there are better ways in writing this.

Code:

#include <iostream>
#include <string>

using namespace std;

int mult(int A[4][4], int B[4][4]){

int C[4][4], num;

for (int i=0; i<4; i++){
     for(int j=0; j<4; j++){
        num = 0;
        for(int k=0; k<4; k++){
          num += A[i][k]*B[k][j];
}
      C[i][j]=num;
      cout << num << " ";
}
     cout << endl;
}

return 0;

}

int main(){

   int A[4][4], ind=0;
cout << "Default Matrix A: \n \n";

   for (int i=0; i<4; i++){
     for(int j=0; j<4; j++){
      A[i][j]=ind; ind++;
      cout<< A[i][j]<< " ";
}
     cout << endl;

}
    cout << "\nMultiplication of A^2: \n \n";
mult(A,A);
return 0;

}

Output:

Default Matrix A:

    0 1 2 3
    4 5 6 7
    8 9 10 11
    12 13 14 15

Multiplication of A^2:

56 62 68 74
152 174 196 218
248 286 324 362
344 398 452 506

Avoid using namespace std

This can cause name collisions because it adds every name in the std namespace to the global namespace. For a small program like this one it's unlikely that you'll run into any problems (then again, maybe not) but it's best to get into the habit of using the std:: prefix on names in the std namespace.

Alternatively, you can introduce using declarations like using std::cout; to add specific names to the global namespace.

Avoid std::endl in favor of \n

std::endl flushes the stream, which can cause a loss in performance.

Declare variables in the most local scope possible

You declare num at the beginning of mult() but you don't actually use it (and initialize it to 0) until you're inside the j loop. It's better to simply declare and initialize it in the same place and in the place where you start to use it.

I would also rename it to be more descriptive (e.g. sum or dot_product).

Avoid hard-coded numbers

Your code works with \$4\times 4\$ matrices but you've got 4 hard-coded all over the place. You say

  This can easily be generalized for any nxn matrix by replacing 4 with any positive number greater than 1.

But there are a lot of instances where 4 needs to be replaced. At the very minimum you should define 4 as a constant and use that constant in the code:

const std::size_t N = 4; // or constexpr instead of const if your compiler supports it

int mult(int A[N][N], int B[N][N]) {
    int C[N][N];

    for (int i = 0; i < N; i++) {
        for (int j = 0; j < N; j++) {
            int num = 0;
            for (int k = 0; k < N; k++) {
                num += A[i][k] * B[k][j];
            }
            C[i][j] = num;
            std::cout << num << " ";
        }
        std::cout << std::endl;
    }

    return 0;
}

Now I just need to change the definition of N once to use a number other than 4.

Use static_assert to enforce the condition that \$N > 1\$

If your compiler supports static_assert you can ensure that \$N > 1\$ at compile time and cause a compilation failure with a simple message to explain the problem.

Function template

The multiplication algorithm is basically the same for any \$N > 1\$, so this function is a good candidate for a function template based on the dimension \$N\$. For example, you have A with N = 4, but in the same program you could have a matrix (2D array) D with N = 3 and the same code is used to multiply with both matrices. Here's a demo using a function template:

#include <iostream>
#include <string>

template<std::size_t N>
int mult(int A[N][N], int B[N][N]) {
    static_assert(N > 1, "N must be greater than 1");

    int C[N][N];

    for (int i = 0; i < N; i++) {
        for (int j = 0; j < N; j++) {
            int num = 0;
            for (int k = 0; k < N; k++) {
                num += A[i][k] * B[k][j];
            }
            C[i][j] = num;
            std::cout << num << " ";
        }
        std::cout << std::endl;
    }

    return 0;
}

int main() {
    const std::size_t N = 4;
    int A[N][N];
    int ind = 0;
    std::cout << "Default Matrix A: \n \n";

    for (int i = 0; i < N; i++) {
        for (int j = 0; j < N; j++) {
            A[i][j] = ind; ind++;
            std::cout << A[i][j] << " ";
        }
        std::cout << std::endl;
    }
    std::cout << "\nMultiplication of A^2: \n \n";
    mult<N>(A, A);

    const std::size_t N2 = 3;
    int D[N2][N2];
    ind = 0;
    std::cout << "\nDefault Matrix D: \n \n";

    for (int i = 0; i < N2; i++) {
        for (int j = 0; j < N2; j++) {
            D[i][j] = ind; ind++;
            std::cout << D[i][j] << " ";
        }
        std::cout << std::endl;
    }
    std::cout << "\nMultiplication of D^2: \n \n";
    mult<N2>(D, D);
    return 0;
}

Consider a Matrix class instead of 2D arrays

Mathematical libraries implement matrices as a class. Internally, the Matrix class may use 2D arrays to store the data, but client code shouldn't depend on how that data is stored. A Matrix class can hide the implementation details from client code.

Extend to non-square matrices

In general, an \$N \times M\$ matrix \$A\$ can be multiplied with a matrix \$B\$ if \$B\$ is \$M \times P\$ (\$A\$ has \$M\$ columns and \$B\$ has \$M\$ rows, but otherwise the two matrices can have different dimensions). With a function template you can easily extend the multiplication function to support non-square matrices. Simply add the necessary template arguments and tweak the algorithm to use dimensions other than N. Here's a demo:

#include <iostream>
#include <string>

template<std::size_t N, std::size_t M, std::size_t P>
int mult(int A[N][M], int B[M][P]) {
    static_assert(N > 1, "N must be greater than 1");
    static_assert(M > 1, "M must be greater than 1");
    static_assert(P > 1, "P must be greater than 1");

    int C[N][P];

    for (int n = 0; n < N; n++) {
        for (int p = 0; p < P; p++) {
            int num = 0;
            for (int m = 0; m < M; m++) {
                num += A[n][m] * B[m][p];
            }
            C[n][p] = num;
            std::cout << num << " ";
        }
        std::cout << std::endl;
    }

    return 0;
}

int main() {
    int A[4][3]{
        {1, 2, 3},
        {4, 5, 6},
        {7, 8, 9},
        {10, 11, 12}
    };

    int B[3][2]{
        {1, 2},
        {3, 4},
        {5, 6}
    };

    mult<4, 3, 2>(A, B);
    return 0;
}

The demo program outputs the \$N \times P\$ (\$4 \times 2\$) matrix product:

22 28
49 64
76 100
103 136


Looking for a more condensed method of assigning values by using arrays and fixed ranges I currently have a Template Workbook set up that has charts and graphs updating when new data is plugged into the Data Tables.
The Data Tables are fixed ranges, and won't change.
The Macro is within the Template Workbook, which is why the below code doesn't set variables for each of the worksheets within the Template Workbook, and Code Names are used for each Worksheet name.
I know the below code is a very basic array code, and works.  However, Is there a more condensed way to perform this array macro?
I have researched, but can't seem to find an example of multiple ranges and assigning values from the Source Workbook - Multiple Worksheets to the Template Workbook - Multiple Worksheets.
Sub Main()

'Turn off screen updates and automatic calculations
    Application.ScreenUpdating = False
    Application.Calculation = xlManual
    Application.DisplayAlerts = False

    Dim wbDataSource As Workbook
    Dim ws As Worksheet

    Set wbDataSource = Workbooks("Segment Trends Data.xlsx")

    For Each ws In wbDataSource.Worksheets
        With ws
            If .Index <> 1 Then
                Dim Data(13) As Variant
                Data(0) = Worksheets("Report1").Range("A1").Value2
                Data(1) = Worksheets("Report1").Range("A5:H11").Value2
                Data(2) = Worksheets("Report2").Range("A1").Value2
                Data(3) = Worksheets("Report2").Range("A5:H11").Value2
                Data(4) = Worksheets("Report3").Range("A1").Value2
                Data(5) = Worksheets("Report3").Range("A5:H11").Value2
                Data(6) = Worksheets("Report4").Range("A1").Value2
                Data(7) = Worksheets("Report4").Range("A5:H11").Value2
                Data(8) = Worksheets("Report5").Range("A1").Value2
                Data(9) = Worksheets("Report5").Range("A5:H11").Value2
                Data(10) = Worksheets("Report6").Range("A1").Value2
                Data(11) = Worksheets("Report6").Range("A5:H11").Value2
                Data(12) = Worksheets("Report7").Range("A1").Value2
                Data(13) = Worksheets("Report7").Range("A5:H11").Value2
            End If
        End With
    Next ws

    wbDataSource.Close SaveChanges:=False

    With wsTTLUSCYTD
        .Range("A1").Value2 = Data(0)
        .Range("A5:H11").Value2 = Data(1)
    End With

    With wsCintiCYTD
        .Range("A1").Value2 = Data(2)
        .Range("A5:H11").Value2 = Data(3)
    End With

    With wsCOLCYTD
        .Range("A1").Value2 = Data(4)
        .Range("A5:H11").Value2 = Data(5)
    End With

    With wsDaytonCYTD
        .Range("A1").Value2 = Data(6)
        .Range("A5:H11").Value2 = Data(7)
    End With

    With wsIndyCYTD
        .Range("A1").Value2 = Data(8)
        .Range("A5:H11").Value2 = Data(9)
    End With

    With wsLouisCYTD
        .Range("A1").Value2 = Data(10)
        .Range("A5:H11").Value2 = Data(11)
    End With

    With wsCoreMktCYTD
        .Range("A1").Value2 = Data(12)
        .Range("A5:H11").Value2 = Data(13)
    End With

    Calculate

    Dim TemplatePath As String
        TemplatePath = "C:\Users\cday\OneDrive - udfinc.com\Budgeting Presentation_Working Files\"
        ActiveWorkbook.SaveAs Filename:=TemplatePath & "Segment Trends - CYTD - Budget Template" & ".xlsm", FileFormat:=52

'Turn screen updates and automatic calculations back on
    Application.ScreenUpdating = True
    Application.Calculation = xlAutomatic
    Application.DisplayAlerts = True

End Sub

Using Debug.Print, the data is pulling from the wbDataSource worksheets within the Loop.

The comment discussion in the original post directly relate to the differences between the ActiveWorkbook and ThisWorkbook. While you code may indeed work, it's not a guarantee. This reference (and this one) have good explanations to illustrate the differences between how you reference a workbook and why you should always pay specific attention to references (whether it's to the Workbook or Worksheet or Range).
As explained in the comments, because you specifically identify the worksheet for all the .Value2 data, you don't need to loop through all the worksheets. This will reduce the setup of your array to
Dim wbDataSource As Workbook
Set wbDataSource = Workbooks("Segment Trends Data.xlsx")

Dim Data(13) As Variant
With wbDataSource
    Data(0) = .Worksheets("Report1").Range("A1").Value2
    Data(1) = .Worksheets("Report1").Range("A5:H11").Value2
    Data(2) = .Worksheets("Report2").Range("A1").Value2
    Data(3) = .Worksheets("Report2").Range("A5:H11").Value2
    Data(4) = .Worksheets("Report3").Range("A1").Value2
    Data(5) = .Worksheets("Report3").Range("A5:H11").Value2
    Data(6) = .Worksheets("Report4").Range("A1").Value2
    Data(7) = .Worksheets("Report4").Range("A5:H11").Value2
    Data(8) = .Worksheets("Report5").Range("A1").Value2
    Data(9) = .Worksheets("Report5").Range("A5:H11").Value2
    Data(10) = .Worksheets("Report6").Range("A1").Value2
    Data(11) = .Worksheets("Report6").Range("A5:H11").Value2
    Data(12) = .Worksheets("Report7").Range("A1").Value2
    Data(13) = .Worksheets("Report7").Range("A5:H11").Value2
End With

Notice that I've changed the With clause to reference the wbDataSource workbook so that's it's very clear where all your data is coming from.
The code you've written actually doesn't need an array, in fact the array obscures what the code is really trying to do. What exactly is Data(0) or Data(11)? You might find it tedious, but I would more clearly define what information all those variables hold. As an example
Dim reportTitle As String
Dim reportDescription As String
Dim regionNumber As Long
Dim regionManager As String

With wbDataSource
    reportTitle = .Worksheets("Report1").Range("A1").Value2
    reportDescription = .Worksheets("Report1").Range("A5:H11").Value2
    regionNumber = .Worksheets("Report2").Range("A1").Value2
    regionManager = .Worksheets("Report2").Range("A5:H11").Value2
    ...
End With

Now when you come back to make an update to your code in six months, you don't have to remember what each of those array slots actually means.
Finally, getting back to the original question of your post, there's no magic or easy way to make the assignments to multiple destination worksheets. You've already defined the code names for each of the sheets, so the VBA you have is about as simple as you can make it. I still recommend the change to more descriptive variable names to make it clear which data you are copying to the destination worksheets.
With wsTTLUSCYTD
    .Range("A1").Value2 = reportTitle
    .Range("A5:H11").Value2 = reportDescription
End With

With wsCintiCYTD
    .Range("A1").Value2 = regionNumber
    .Range("A5:H11").Value2 = regionManager
End With