Coding Challenge: Return The Smaller String A coding challenge in which we are to write a function that compares two strings and returns the one that is smaller. The comparison is both lexicographical and numerical, depending on the content of the strings as explained below and in the comments of the code.

Both strings may contain any characters. Consecutive numbers in the string are considered a single number. If during the search the character of only one string is a number, then that string is returned as numbers are lexicographically smaller than letters.

If during the search the characters of both strings are numbers, then parseInt(strX.slice(i)) checks if there are more consecutive digits in each string and returns the string whose number is numerically smaller.

Examples:

input: "a", "b"
expected output: "a" since "a" comes before "b" alphabetically

input: "a1", "a2"
expected output: "a1" since 1 comes before 2

input: "a10", "a2"
expected output: "a2" since 2 comes before 10

Here is the code:

const smallestString = (str1, str2) => {
    // we only need to iterate through the shortest string
    const len = str1.length < str2.length ? str1.length : str2.length;
    for (let i = 0; i < len; i++) {
        // check if both letters are strings
        if (str1[i].toUpperCase() !== str1[i].toLowerCase() && str2[i].toUpperCase() !== str2[i].toLowerCase()) {
            if (str1[i] === str2[i]) { // if both letters are the same, continue
                continue;
            } else {
                return str1[i] < str2[i] ? str1 : str2; // otherwise return the string with the 'smaller' char at that index
            }
        } else if (!isNaN(str1[i] || !isNaN(str2[i]))) { // check if either char is a number
            if (!isNaN(str1[i]) && !isNaN(str2[i])) { // if both chars are numbers, return str with numerically smaller number
              return parseInt(str1.slice(i)) < parseInt(str2.slice(i)) ? str1 : str2;
            }
        } else {
            return str1[i] < str2[i] ? str1 : str1;
        }
    }
    return str1; // if we get here then both strings are equal and either can be returned
}

console.log(smallestString('a10', 'a2')); // returns 'a2'

I am seeking any and all feedback about cleaning up the code and possibly improving the algorithm's current time complexity of O(n).

if (!isNaN(str1[i]) && !isNaN(str2[i])) { // if both chars are numbers, return str with numerically smaller number
          return parseInt(str1.slice(i)) < parseInt(str2.slice(i)) ? str1 : str2;
        }

seems like a bug. In case the numbers compare equal the code blindly returns the first string. Consider (a10c, a10b).

return str1; // if we get here then both strings are equal and either can be returned

seems like another bug. At this point we only know that the strings are equal up to the length of the shortest one, and the shortest one should be returned. Which one is shortest is not tested here.

The overall logic looks overcomplicated. Consider testing for numbers first; that would eliminate the need for a confusing toUpper/toLower mess, and the special case of special characters.


Script to convert Shift + Spacebar into four spaces on SE I've posted this feature request and would like to improve the code involved to handle a few "bugs" (not that it's necessary to provide such code, just for my own practice), but I doubt I'm doing it as efficiently as possible with my updated conditions for the feature:

$("textarea").keydown(function (e) {
    if (e.shiftKey && e.keyCode == 32) {
        var white = " ",
            pad = "    ",
            t = this.value,
            n = this.selectionStart,
            s = this.scrollTop,
            r = [t.slice(0, n), t.slice(n)];
        if (r[0].indexOf(white, r[0].length - 1) !== -1) {
            this.value = r.join(pad);
            this.selectionStart = this.selectionEnd = n + pad.length;
            this.scrollTop = s;
            e.preventDefault();
        }
    }
});

Purpose:

Most importantly, Shift + Spacebar creates 4 spaces, cancelling the default space action.
Also to avoid accidental usage, it should not execute unless the character before the cursor is a space, or empty. (Avoids "XML is..." becoming "XML    is..."). I couldn't figure out how to check for either whitespace OR a new line, so if someone could mention how that's done, it would be helpful, but if not I can always ask on SO.
To avoid a (reported) potential bug:

  the scrollTop property of long textareas must be involved because re-setting the .value to insert the tab can cause a scroll [in some browsers]

Review

I'm looking for general improvements, best practices, shortening opportunities, simplification, etc. I'd particularly like to learn a better method / trick for figuring out if the previous character doesn't contain a non-space character rather than checking for either nothing or a space.

You can use charAt to grab the preceding character and check it against a space, newline, or empty string (the case when the cursor is at the start). But let's not stop there.

white and pad are okay enough (white is probably unnecessary, but space would be a better name since newlines are typically included in the generic term "whitespace"), but those other names are truly horrible!
You should probably maintain their full text selection--beginning and end.

Updated code:

$("textarea").keydown(function (e) {
    if (e.shiftKey && e.keyCode === 32) {
        var pad = "    ",
            text = this.value,
            start = this.selectionStart,
            end = this.selectionEnd,
            scroll = this.scrollTop,
            before = text.slice(0, start),
            after = text.slice(start),
            prevChar = before.charAt(before.length - 1);
        if (prevChar === "" || prevChar === " " || prevChar === "\n") {
            this.value = before + pad + after;
            this.selectionStart = start + pad.length;
            this.selectionEnd = end + pad.length;
            this.scrollTop = scroll;
            e.preventDefault();
        }
    }
});

Here are some future enhancement ideas:

Search for the preceding newline to ensure there is only whitespace before the cursor. This would fully stop expansion inside a sentence. Right now you can still insert four spaces in the middle right after a space.
Consider using ctrl + space so you can add ctrl + shift  + space for outdenting.
Shift/unshift every selected line instead of just the cursor position. Right now the { } button can be wrangled to do shifting, but it's cumbersome since you have to put some non-space character on the first line to get indentation.


Transforming UCI-move into bitboard, chess I want to transform a UCI-move into bitboard.
for example a2a3 -> 32768, 8388608
I need to assign [7,6,...,0] to [a,b,...,h] so that for each letter i have the assigned number(n) to calculate 2^n
Which I can then left shift by the value in uci[1] or uci[3] *8 depending on start- or endfield.
This is my approach and it doesn't look very nice and redundant.
def ucitoBit(uci):
    if uci[0] == 'a':
        mask1 = 2 ** 7
    if uci[0] == 'b':
        mask1 = 2 ** 6
    if uci[0] == 'c':
        mask1 = 2 ** 5
    if uci[0] == 'd':
        mask1 = 2 ** 4
    if uci[0] == 'e':
        mask1 = 2 ** 3
    if uci[0] == 'f':
        mask1 = 2 ** 2
    if uci[0] == 'g':
        mask1 = 2 ** 1
    if uci[0] == 'h':
        mask1 = 2 ** 0
    mask1 = mask1 << 8 * (int(uci[1]) - 1)

    if uci[2] == 'a':
        mask2 = 2 ** 7
    if uci[2] == 'b':
        mask2 = 2 ** 6
    if uci[2] == 'c':
        mask2 = 2 ** 5
    if uci[2] == 'd':
        mask2 = 2 ** 4
    if uci[2] == 'e':
        mask2 = 2 ** 3
    if uci[2] == 'f':
        mask2 = 2 ** 2
    if uci[2] == 'g':
        mask2 = 2 ** 1
    if uci[2] == 'h':
        mask2 = 2 ** 0
    mask2 = mask2 << 8 * (int(uci[3]) - 1)
    bitstring = [np.uint64(mask1), np.uint64(mask2)]
    return bitstring

Option 1:
The most basic thing I can suggest is that if you have a mapping from one thing to another, and if that mapping is fixed, you should be using a mapping data type.
The mapping data type in python is the dict, which can be written in-line like so:
uci_bit = {
    'a': 2**7, 'b': 2**6, 'c': 2**5, 'd': 2**4,
    'e': 2**3, 'f': 2**2, 'g': 2**1, 'h': 2**0,
}

Alternatively, the dict() function returns a dictionary, and takes keyword arguments:
uci_bit = dict(a=2**7, b=2**6, c=2**5, d=2**4,
               e=2**3, f=2**2, g=2**1, h=2**0)

Same result either way. You could then access the dict using the standard square-bracket notation:
ch = uci[0]
mask1 = uci_bit[ch]

Or as a single expression:
mask1 = uci_bit[uci[0]]

Option 2:
However, there's another approach that has some potential. Looking things up in a dictionary has a cost. It's technically \$O(n)\$ but maybe there's a constant or two in there. So ...
Because the two components of the uci location are fixed-width ('a' and '2') you might be able to look them up in a string using the str.index method and get better performance.
Note: I wrote "might be". The way to know for sure is to write both bits of code and run a timing test on the alternatives. Look up the timeit module in the standard distribution.
Something like:
UCI_CHARS = 'hgfedcba'

mask1 = 2 ** UCI_CHARS.index(uci[0])
mask2 = 2 ** UCI_CHARS.index(uci[2])

You might also want to check the result of using dict versus str.index versus the int() function on the single-digit part of uci decoding.
Option 3:
It's not clear how many of these operations you are performing. If you are doing a single game, and there might be two dozen uci to bit operations, then it probably doesn't matter. But if you're doing a lot of games at the same time, it might behoove you to generate a dictionary containing all the UCI letter/number combinations, and do the lookup as a single operation. Something like:
for ch in 'abcdefgh':
    for digit in '12345678':
        uci = ch + digit
        bit = # whatever computation
        uci_bit[uci] = bit

This would let you decode 'a2' -> bits in a single step at the cost of the up-front loop.
Other considerations
I asked the Duck about bitboard, and was immediately presented with the fact that there is no single standard representation. Instead, there are different representations for different purposes and different CPU architectures.
A result of this is that you should definitely be doing this inside a class or function, and there should definitely be a "name" given to the kind of bitboard you want to compute, so that readers will understand which of the several options you are choosing. Rank-major versus File-major and big- versus little- endian representations are all valid, so make sure to include this information either in the class or function name, or in the commentary.
def uci_to_bitboard_lsf(uci: str) -> int:
    """ Convert UCI notation to bitboard(LSF, little-endian) """

    ...

Also, I suspect that using * may be a bug, and you should be using + instead. (It really should be a bitwise or, the | operator, but + will work.)


C# REST / Http Client Abstraction I have a REST library. These are the abstractions. Have I missed anything that a developer would need to use this in a dependency injection / IoC Container scenario? Anything that bothers you about this?

All Code Here

public interface IRestClient
{
    IRestHeadersCollection DefaultRequestHeaders { get; }
    TimeSpan Timeout { get; set; }
    Task DeleteAsync(Uri queryString);
    Task DeleteAsync(Uri queryString, CancellationToken cancellationToken);
    Task<RestResponse<TReturn>> GetAsync<TReturn>();
    Task<RestResponse<TReturn>> GetAsync<TReturn>(Uri queryString);
    Task<RestResponse<TReturn>> GetAsync<TReturn>(Uri queryString, CancellationToken cancellationToken);
    Task<RestResponse<TReturn>> PatchAsync<TReturn, TBody>(TBody body, Uri queryString);
    Task<RestResponse<TReturn>> PatchAsync<TReturn, TBody>(TBody body, Uri queryString, CancellationToken cancellationToken);
    Task<RestResponse<TReturn>> PostAsync<TReturn, TBody>(TBody body, Uri queryString);
    Task<RestResponse<TReturn>> PostAsync<TReturn, TBody>(TBody body, Uri queryString, CancellationToken cancellationToken);
    Task<RestResponse<TReturn>> PutAsync<TReturn, TBody>(TBody body, Uri queryString);
    Task<RestResponse<TReturn>> PutAsync<TReturn, TBody>(TBody body, Uri queryString, CancellationToken cancellationToken);
}

public interface IRestClientFactory
{
    IRestClient CreateRestClient(Uri baseUri);
    IRestClient CreateRestClient();
}

public interface IRestHeadersCollection : IEnumerable<KeyValuePair<string, IEnumerable<string>>>
{
    void Add(string name, string value);
    void Add(string name, IEnumerable<string> values);
    void Clear();

    IEnumerable<string> this[string name]
    {
        get;
    }

    bool Contains(string name);
}

public partial interface ISerializationAdapter
{
    /// <summary>
    /// Takes an object of Type T and converts it to binary data
    /// </summary>
    Task<byte[]> SerializeAsync<T>(T value);

    /// <summary>
    /// Takes binary data and converts it to an object of type T
    /// </summary>
    Task<T> DeserializeAsync<T>(byte[] data);
}

public interface ITracer
{
    void Trace(HttpVerb httpVerb, Uri baseUri, Uri queryString, byte[] body, TraceType traceType, int? httpStatusCode, IRestHeadersCollection restHeadersCollection);
}

public class RestResponse<TBody> : RestResponse
{
    public TBody Body { get; }

    public RestResponse(TBody body, IRestHeadersCollection restHeadersCollection, int statusCode, object underlyingResponse) : base(restHeadersCollection, statusCode, underlyingResponse)
    {
        Body = body;
    }

    #region Implicit Operator
    public static implicit operator TBody(RestResponse<TBody> readResult)
    {
        return readResult.Body;
    }

    public TBody ToTBody()
    {
        return Body;
    }
    #endregion
}

public class RestResponse
{
    #region Public Properties
    public int StatusCode { get; }
    public object UnderlyingResponse { get; }
    public IRestHeadersCollection Headers { get; }
    #endregion

    #region Constructor
    public RestResponse(IRestHeadersCollection restHeadersCollection, int statusCode, object underlyingResponse)
    {
        StatusCode = statusCode;
        UnderlyingResponse = underlyingResponse;
        Headers = restHeadersCollection;
    }
    #endregion
}

Originally I thought it might be useful to allow task async calls for serialization/deserialization but I've come to think that this is a bit redundant... Any thoughts on this?

IRestClient looks over-engineered. All of those helper members merge down to one common Call which can make the interface much simpler to define.

public interface IRestClient {
    IRestHeadersCollection DefaultRequestHeaders { get; }
    string DefaultContentType { get; set; }
    TimeSpan Timeout { get; set; }
    Task<RestResponse<TReturn>> SendAsync<TReturn>(RestRequest request, CancellationToken cancellationToken = default(CancellationToken));
}

The client now simplified to its core features.

The commonly used parameters can be aggregated into a class

public class RestRequest {

    public RestRequest() {
    }

    public RestRequest(IRestClient client) : this() {
        var contentType = client.DefaultContentType;
        if (!string.IsNullOrEmpty(contentType)) {
            ContentType = contentType;
        }
        var headers = client.DefaultRequestHeaders;
        if (headers != null) {
            foreach (var header in headers) {
                Headers.Add(header.Key, header.Value);
            }
        }
    }

    public IRestHeadersCollection Headers { get; } = new RestRequestHeaders();

    public Uri Resource { get; set; }
    public HttpVerb HttpVerb { get; set; } = HttpVerb.Get;
    public string ContentType { get; set; } = "application/json";
    public object Body { get; set; }

    class RestRequestHeaders : HttpHeaders, IRestHeadersCollection {
        public RestRequestHeaders() : base() { }
        public IEnumerable<string> this[string name] => GetValues(name) ?? Array.Empty<string>();
    }
}

Naming is important here. Change the name to resource from queryString, which means something different to how it was originally being used with regards to HTTP requests.

Parameter order should also be changed to be more consistent.

Your original call would have looked something like

...await client.PostAsync<ReturnObject, BodyObject>(body, "...");

with the URI parameter second. When making the request, the first argument should be the resource being called. Everything else can be built up from there

...await client.PostAsync<ReturnObject, BodyObject>("...", body);
...await client.PostAsync<ReturnObject, BodyObject>("...", body, token);
//...etc

The following extensions allow for the same functionality as before on the IRestClient abstraction

public static class RestClientExtensions {
    #region Delete
    public static Task DeleteAsync(this IRestClient client, string resource) {
        return client.DeleteAsync(new Uri(resource, UriKind.Relative));
    }

    public static Task DeleteAsync(this IRestClient client, Uri resource) {
        return client.DeleteAsync(resource, CancellationToken.None);
    }

    public static Task DeleteAsync(this IRestClient client, Uri resource, CancellationToken cancellationToken) {
        var request = new RestRequest(client) {
            Resource = resource,
            HttpVerb = HttpVerb.Delete
        };
        return client.SendAsync<object>(request, cancellationToken);
    }
    #endregion

    #region Get
    public static Task<RestResponse<TReturn>> GetAsync<TReturn>(this IRestClient client) {
        var request = new RestRequest(client) {
            HttpVerb = HttpVerb.Get
        };
        return client.SendAsync<TReturn>(request, CancellationToken.None);
    }

    public static Task<RestResponse<TResult>> GetAsync<TResult>(this IRestClient client, string resource) {
        try {
            return client.GetAsync<TResult>(new Uri(resource, UriKind.Relative));
        } catch (UriFormatException ufe) {
            if (ufe.Message == "A relative URI cannot be created because the 'uriString' parameter represents an absolute URI.") {
                throw new UriFormatException(Messages.ErrorMessageAbsoluteUriAsString, ufe);
            }
            throw;
        }
    }

    public static Task<RestResponse<TReturn>> GetAsync<TReturn>(this IRestClient client, Uri resource) {
        return client.GetAsync<TReturn>(resource, CancellationToken.None);
    }

    public static Task<RestResponse<TReturn>> GetAsync<TReturn>(this IRestClient client, Uri resource, CancellationToken cancellationToken) {
        var request = new RestRequest(client) {
            Resource = resource
        };
        return client.SendAsync<TReturn>(request, cancellationToken);
    }
    #endregion

    #region Patch
    public static Task<RestResponse<TReturn>> PatchAsync<TReturn, TBody>(this IRestClient client, Uri resource, TBody body) {
        return client.PatchAsync<TReturn, TBody>(resource, body, CancellationToken.None);
    }

    public static Task<RestResponse<TReturn>> PatchAsync<TReturn, TBody>(this IRestClient client, Uri resource, TBody body, CancellationToken cancellationToken) {
        var request = new RestRequest(client) {
            Resource = resource,
            HttpVerb = HttpVerb.Patch,
            Body = body,
        };
        return client.SendAsync<TReturn>(request, cancellationToken);
    }
    #endregion

    #region Post
    public static Task<RestResponse<TReturn>> PostAsync<TReturn, TBody>(this IRestClient client, TBody body, Uri querresourceString) {
        return client.PostAsync<TReturn, TBody>(body, querresourceString, CancellationToken.None);
    }

    public static Task<RestResponse<TReturn>> PostAsync<TReturn, TBody>(this IRestClient client, TBody body, Uri resource, CancellationToken cancellationToken) {
        var request = new RestRequest(client) {
            Resource = resource,
            HttpVerb = HttpVerb.Post,
            Body = body,
        };
        return client.SendAsync<TReturn>(request, cancellationToken);
    }
    #endregion

    #region Put
    public static Task<RestResponse<TReturn>> PutAsync<TReturn, TBody>(this IRestClient client, TBody body, Uri resource) {
        return client.PutAsync<TReturn, TBody>(body, resource, CancellationToken.None);
    }

    public static Task<RestResponse<TReturn>> PutAsync<TReturn, TBody>(this IRestClient client, TBody body, Uri resource, CancellationToken cancellationToken) {
        var request = new RestRequest(client) {
            Resource = resource,
            HttpVerb = HttpVerb.Put,
            Body = body,
        };
        return client.SendAsync<TReturn>(request, cancellationToken);
    }
    #endregion
}


Minimum number of flight segments using breadth-first search I'm practising with graphs, and trying to solve a problem of calculating the minimum number of flight segments, applying breadth-first search.

The code is working, but I think, that it's not clean.

Can anyone suggest how I refactor it to make it cleaner?

def distance(adj, s, t):

    n = len(adj)
    queue = []
    visited = set()
    path = []

    queue.append([s])

    dist = 0

    while (len(queue) > 0):
        path = queue.pop(0)

        last_vertex = path[-1]

        if last_vertex == t:
            # print(path)
            dist = len(path)-1

        elif last_vertex not in visited:
            for w in adj[last_vertex]:
                new_path = list(path)
                new_path.append((w))
                queue.append(new_path)
            visited.add(last_vertex)

    if dist != 0:
        return dist
    else:
        return -1

if __name__ == '__main__':
    input = sys.stdin.read()
    data = list(map(int, input.split()))
    n, m = data[0:2]
    data = data[2:]
    edges = list(zip(data[0:(2 * m):2], data[1:(2 * m):2]))
    adj = [[] for _ in range(n)]
    for (a, b) in edges:
        adj[a - 1].append(b - 1)
        adj[b - 1].append(a - 1)
    s, t = data[2 * m] - 1, data[2 * m + 1] - 1

    print(distance(adj, s, t))

I represent graph in such way. The first line contains non-negative integers n and m — the number of vertices and the number of edges respectively. The vertices are always numbered from 1 to n. Each of the following m lines defines an edge in the format u v where 1 ≤ u, v ≤ n are endpoints of the edge :

4 5
2 1
4 3
1 4
2 4
3 2
1 3

The last two digits stands for two vertices, we need to find path between.

1. Review

There's no docstring. What does this function do? What arguments does it take? What does it return?
The code uses a list to represent the queue of nodes to be visited. But if you look at the TimeComplexity page on the Python wiki, you'll see that list.pop(0) takes time proportional to the length of the list, and that means that the algorithm takes time that's quadratic in the number of nodes in the graph.

Instead of keeping the queue of nodes in a list, you need a collections.deque, which has an efficient popleft method.
Each time the code reaches a new node, it copies the path to reach that node, and puts the path on the queue. But this is wasteful of memory (because paths might become long and have to be copied out many times) and leads to quadratic runtime.

Since the code only needs to return the distance to the target (not the path), it only needs to store the distance to each node.
The variable n is not used.
Instead of:

while (len(queue) > 0):

write:

while queue:

It would be faster to check whether a node has been visited before adding it to the queue (rather than after removing it from the queue). This avoids unnecessary append and pop operations on visited nodes.
It would be faster to check whether the target has been reached before adding it to the queue (rather than after removing it from the queue). This avoids having to wait for the target to move all the way up the queue.
distance returns -1 if there was no path from source to target. But it is risky to return an exceptional value like this: it would be all too easy for the caller to forget to check the result. It is better to raise an exception for exceptional cases.

2. Revised code

from collections import deque

class Unreachable(Exception):
    """Exception raised when there is no path between nodes in a graph."""

def distance(graph, source, target):
    """Return the number of edges in the shortest path from source to
    target in a directed graph. The graph must be represented as a
    mapping from a node to a collection of its neighbours. If there is
    no path from source to target, raise Unreachable.

    >>> distance({1: [2, 3], 2: [3, 4], 3: [4, 5]}, 1, 5)
    2

    """
    if source == target:
        return 0
    queue = deque([(source, 0)]) # Queue of pairs (node, distance).
    visited = set()              # Set of visited nodes.
    while queue:
        node, dist = queue.popleft()
        visited.add(node)
        for neighbour in graph[node]:
            if neighbour in visited:
                continue
            elif neighbour == target:
                return dist + 1
            else:
                queue.append((neighbour, dist + 1))
    else:
        raise Unreachable("No path from {!r} to {!r}".format(source, target))


What do you think of my EventAggregator implementation in C#? I was looking for a simple Event Aggregator to use in my app, but I found people get very complicated very quickly, so I thought this was easy enough to write myself. It seems to work, but I was wondering what you think and if you would change or add anything. (I know some of them raise events back on the same thread that it was registered, but for what I was planning I don't need this.)

public class EventAggregator
{
    private Dictionary<Type, List<WeakReference>> _listeners;

    private object _syncLock;

    public EventAggregator()
    {
        _listeners = new Dictionary<Type, List<WeakReference>>();
        _syncLock = new object();
    }

    public EventAggregator Register<TEvent>(Action<TEvent> eventAction)
    {
        LockAround(() =>
            {
                var listeners = GetListeners(typeof(TEvent));
                listeners.RemoveAll(wr => !wr.IsAlive || (Action<TEvent>)wr.Target == eventAction);
                listeners.Add(new WeakReference(eventAction));
            });
        return this;
    }

    public void Raise<TEvent>(TEvent ev)
    {
        List<WeakReference> targets = null;
        LockAround(() =>
            {
                var listeners = GetListeners(typeof(TEvent));
                targets = listeners
                    .Where(wr => wr.IsAlive)
                    .ToList();
            });

        targets.ForEach(wr => ((Action<TEvent>)wr.Target)(ev));
    }

    private void LockAround(Action action)
    {
        lock (_syncLock)
        {
            action();
        }
    }

    private List<WeakReference> GetListeners(Type type)
    {
        List<WeakReference> result;
        if (_listeners.TryGetValue(type, out result))
        {
            return result;
        }
        result = new List<WeakReference>();
        _listeners[type] = result;
        return result;
    }
}

This is how I plan to use it:

class Program
{
    static void Main(string[] args)
    {
        var ea = new EventAggregator();

        var view = new View(ea);
        var p = new Presenter(ea);

        view.Name = "Name1";
        view.Name = "Name2";

        Console.ReadKey();
    }
}

interface INameChangeEvent
{
    string OldName { get; set; }

    string NewName { get; set; }
}

public class NameChangeEvent : EventArgs, INameChangeEvent
{
    public NameChangeEvent(string oldName, string newName)
    {
        this.OldName = oldName;
        this.NewName = newName;
    }

    public string OldName { get; set; }

    public string NewName { get; set; }
}

class View
{
    private string _name;
    private EventAggregator _ea;

    public View(EventAggregator ea)
    {
        _ea = ea;
    }

    public string Name
    {
        get
        {
            return _name;
        }
        set
        {
            _ea.Raise<INameChangeEvent>(new NameChangeEvent(_name, value));
            _name = value;
        }
    }
}

class Presenter
{
    public Presenter(EventAggregator ea)
    {
        ea.Register<INameChangeEvent>((ev) =>
            Console.WriteLine(string.Format("View name was changed from {0} to {1}",
              ev.OldName, ev.NewName)));
    }
}

I haven't thoroughly looked at it but these are my initial thoughts.

Minor points

I would initialise the private variables inline like this private object _syncLock = new Object(); because they don't depend on parameters given to the constructor.

For readability I wouldn't use the LockAround method, I would just lock _syncObject.

In GetListeners is there any reason you are adding a list to the dictionary when no events have been registered for that type? Also, you could make it generic by pulling in the typeof from Raise like so _listeners.TryGetValue(typeof(TEvent), out result).

In Register<TEvent> what is the purpose of (Action<TEvent>)wr.Target == eventAction? I think if someone wants to register the same event handler twice then they should be able to.

You might want to make your event aggregator a singleton or use a dependency injection framework to ensure there is only ever one instance in existence.

Worries

In Raise<TEvent> the following line worries me targets.ForEach(wr => ((Action<TEvent>)wr.Target)(ev));. I'm not sure there is any guarantee the weak reference will still be alive at this point. You probably want to solidify the reference and check that it is not null before calling it.

Finally

Have you looked at Microsoft's event aggregator and unity dependency injection framework? I think they probably do what you are looking for and will save you from reinventing the wheel.


Pyramid of doom Grape endpoints I'm creating an API endpoint to create and delete associations, essential creating and deleting records of a join model.

The code below doesn't seem idiomatic or pragmatic at all. What's the best way to create 2 endpoints, POST and DELETE for the same route?

module API
  module V1
    class UserActivities < Grape::API
      include API::V1::Defaults

      format :json

      namespace :users do

        route_param :user_id do
          resource :activities do
            desc 'Create User + Activity associations'
            route_param :activity_id do
              post do
                #pyramid of doom
              end
            end
          end

          resource :activities do
            desc 'Delete User + Activity associations'
            route_param :activity_id do
              delete do
                #pyramid of doom
              end
            end
          end
        end
      end
    end
  end
end

This generates the following routes:

   POST  |  /api/:version/users/:user_id/activities/:activity_id(.:format)  |  v1  |  Create User + Activity associations
 DELETE  |  /api/:version/users/:user_id/activities/:activity_id(.:format)  |  v1  |  Delete User + Activity associations

But is there a better way to do this using Grape?

You don't have to define the whole flow per route, this still leads you to the pyramid of hell.

  namespace :users do
    route_param :user_id do
      resource :activities do
        desc 'Create User + Activity associations'
        route_param :activity_id do
          post do
            #pyramid of doom
          end

          delete do
            #pyramid of doom
          end
        end
      end
    end
  end

However, you can register parts of the route in different files

namespace :users do
  register UserActivities
end

Then, within user_activities.rb you can add the rest of the code


"Vc" as in Vancouver, or Valencia? It is common to shorten city names when writing, or at least in Sweden.

"Göteborg" (Gothenburg) is often referred to as "Gbg"
"Uddevalla" is commonly written as "Ua"
"Trollhättan" is often written as "Thn"
...and a whole lot of other city names are often shortened

"Uddevalla" could also be shortened as "Uva", "Uvalla", or even "Udl" (although I've never seen anyone actually type that, according to my rules here it would be allowed). "vd" however would not be allowed as the letters must appear in order, "dv" would be allowed as an abbreviation for "Uddevalla" as "v" comes after "d".

Given a list of cities (or more generally: Strings) and a search string, print the city names that can be constructed by using these abbreviation rules.

Example cities (my townnames.txt file):

Strömstad
Skee
Överby
Tanum
Rabbalshede
Hällevadsholm
Dingle
Munkedal
Uddevalla
Ljungskile
Svenshögen
Stenungsund
Stora Höga
Kode
Ytterby
Göteborg
Trollhättan
Vänersborg
Varberg
Stockholm
Simrishamn

These are cities in Sweden, mostly in around my area. With the above list and the search string "Sm" the following cities are printed:

["Strömstad", "Stockholm", "Simrishamn"]

Instead given the string "Vbg" the results are:

["Vänersborg", "Varberg"]

After looking at the review I got from my previous Haskell question I wrote the following code to accomplish this:

module Main
   where

nameMatch :: [Char] -> [Char] -> Bool
nameMatch [] [] = True
nameMatch s [] = False
nameMatch [] s = True
nameMatch search@(s:restSearch) (c:city)
   | c == s    = nameMatch restSearch city
   | otherwise = nameMatch search city

main = do
  putStrLn "Loading list of towns..."
  text <- readFile "townnames.txt"
  putStrLn "Enter search phrase:"
  search <- getLine
  let cities = lines text
  putStrLn $ show $ filter (nameMatch search) cities

Any possible improvements or suggestions welcome.

[Char] is usually written as String.

You have a redundant base case: nameMatch [] [].

The parameter names are a bit off.  A common convention for naming the head and tail is x:xs.

nameMatch :: String -> String -> Bool
nameMatch [] _      = True
nameMatch abbrev [] = False
nameMatch abbrev@(a:as) word@(w:ws)
   | a == w    = nameMatch as ws
   | otherwise = nameMatch abbrev ws

For printing the output, if you want each result to be on a separate line rather than as a list (which would produce output like ["Trollh\228ttan"]), then write

mapM putStrLn $ filter (nameMatch search) cities


Idiomatic Django conditional filter queryset I am sharing a viewset between two types of routes in Django: one is a nested route (/users/:id/comments/:id/), and a regular route (/comments/:id/). The query set returned is either filtered by the user_pk or it's not. What is an idiomatic way of writing the following?

if(user_pk):
    queryset = Comment.objects.filter(pk=pk, user=user_pk)
else:
    queryset = Comment.objects.filter(pk=pk)

It's fine.
As this function doesn't have a default value for parameters,
you would have to use a dictionary if you want to make it more DRY.
I'd do something like:

kwargs = {
    'pk': pk,
    'user': user_pk
}
if not kwargs['user']:
    del kwargs['user']
queryset = Comment.objects.filter(**kwargs)


Large find/replace in Word using VBA I have a large amount of data from a hardware device that needs to be cleaned up so it can be analyzed in Excel.

I wrote a simple find/replace VBA macro in word to convert it to .csv, but the macro takes a long time to run through large sets of data.

I'm looking for suggestions on a faster way to code this. Alternately, if there is another option for cleaning the data that would work better than Word and VBA, I'm willing to switch to it.

Sub CleanText()

Application.ScreenUpdating = False

' clean data for import to excel
'
Selection.Find.ClearFormatting
Selection.Find.Replacement.ClearFormatting
With Selection.Find
    .Text = "^p" & "^p"
    .Replacement.Text = "wvw"
    .Forward = True
    .Wrap = wdFindContinue
    .Format = False
    .MatchCase = False
    .MatchWholeWord = False
    .MatchWildcards = False
    .MatchSoundsLike = False
    .MatchAllWordForms = False
End With
Selection.Find.Execute Replace:=wdReplaceAll

Selection.Find.ClearFormatting
Selection.Find.Replacement.ClearFormatting
With Selection.Find
   .Text = "^p"
    .Replacement.Text = ","

End With
Selection.Find.Execute Replace:=wdReplaceAll

Selection.Find.ClearFormatting
Selection.Find.Replacement.ClearFormatting
With Selection.Find
   .Text = "wvw"
    .Replacement.Text = "^p"
    .Forward = True

End With
Selection.Find.Execute Replace:=wdReplaceAll

Selection.Find.ClearFormatting
Selection.Find.Replacement.ClearFormatting
With Selection.Find
   .Text = "$GPGGA" & "(*)" & "$GPRMC"
    .Replacement.Text = "$GPRMC"
    .MatchWildcards = True

End With
Selection.Find.Execute Replace:=wdReplaceAll

Selection.Find.ClearFormatting
Selection.Find.Replacement.ClearFormatting
With Selection.Find
   .Text = "W" & "(*)" & "S"
    .Replacement.Text = "W,"
End With
Selection.Find.Execute Replace:=wdReplaceAll

Selection.Find.ClearFormatting
Selection.Find.Replacement.ClearFormatting
With Selection.Find
   .Text = "V" & "(*)" & "S"
    .Replacement.Text = "V,,,,,"
End With
Selection.Find.Execute Replace:=wdReplaceAll

Selection.Find.ClearFormatting
Selection.Find.Replacement.ClearFormatting
With Selection.Find
   .Text = "$GPRMC,"
    .Replacement.Text = ""
End With
Selection.Find.Execute Replace:=wdReplaceAll

Application.ScreenUpdating = True

End Sub

You're doing a lot with .Selection. By default, that is going to be slow. Doing things behind the scenes is always faster. Either way, working with selection isn't the best way to go. Either pass the document to the sub or prompt the user. Is the selection a document, text, body, header, title, section? Be explicit.

The way you use .Find makes me thing you're referring to a range, which can be any of those things. Let's take a look at the basic skeleton of your sub -

    With Selection
    .Find.ClearFormatting
    .Find.Replacement.ClearFormatting
    .Find
    .Find.Execute
    .Find.ClearFormatting
    .Find.Replacement.ClearFormatting
    .Find
    .Find.Execute
    .Find.ClearFormatting
    .Find.Replacement.ClearFormatting
    .Find
    .Find.Execute
    .Find.ClearFormatting
    .Find.Replacement.ClearFormatting
    .Find
    .Find.Execute
    .Find.ClearFormatting
    .Find.Replacement.ClearFormatting
    .Find
    .Find.Execute
    .Find.ClearFormatting
    .Find.Replacement.ClearFormatting
    .Find
    .Find.Execute
    .Find.ClearFormatting
    .Find.Replacement.ClearFormatting
    .Find
    .Find.Execute
End With

Yikes. You're doing the same thing 7 times. At worst that should be refactored. Something like this -

Private Sub AlterRange(ByVal target As Range, ByVal findString As String, ByVal replaceString As String)
    With target
        .Find.ClearFormatting
        .Find.Replacement.ClearFormatting
        .Find
            .Text findString
            .Replacement replaceString
        .Find.Execute Replace:=wdReplaceAll
    End With
End Sub

Now something like this would work

Dim target As Range
Set target = Selection
Dim findStrings(1 To 7) As String
findStrings(1) = "^p^p"
findStrings(2) = "^p"
findStrings(3) = "wvw"
findStrings(4) = "$GPGGA" & "(*)" & "$GPRMC"
'...
Dim replacestrings(1 To 7) As String
replacestrings(1) = "wvw"
replacestrings(2) = ","
'...

Dim i As Long
For i = 1 To 7
    AlterRange target, findStrings(i), replacestrings(i)
Next

All of this is untested, but you get the idea.

What you could do to speed it up is to create a variant (or whatever) to store the entire selection in and then do your finding and replacing in the variant, then place the variant back out to the sheet overwriting the selection.

I don't know how all this would interact with your formatting, but it doesn't seem like you're keeping any formatting so it shouldn't be a problem.


Generating and storing a unique salt in Oracle database using Java This is my account table:

CREATE TABLE mwUser (
  username varchar2(20) primary key not null,
  salt varchar2(64) unique not null,
  hashedpw varchar2(64) not null,
  email varchar2(320) unique not null
);

When I register a new user with my java webservice I automatically generate a random salt and store in the database base (see code below):

Connection c = dataSource.getConnection();
Statement stmt = c.createStatement();
boolean isFound=true;
String randomSalt="";
while(isFound){
   isFound=false;
   String randomSalt=createRandomString(); //creates salt/random string
   ResultSet rs = stmt.executeQuery("SELECT * FROM mvUser where salt="+randomString);
   //check if salt is unique
   while (rs.next()) {
     isFound=true;
   }
}

Statement insertStmt = c.createStatement();
String hashedPW=hash(randomSalt+pw); //generates hashed pw
ResultSet rs = stmt.executeQuery("INSERT INTO mvUser VALUES(.....));

stmt.close();
c.close();

But as you can see, from my point of view my code is not clean/performant/readable because I have two statements (for checking if salt is unique and and second statement is for inserting).

How can I automatically generate an unique string(salt) and hash it at the same time. I am trying to make my code more performant and read able.

Please use best practices in general when dealing with password hashing.

If you have to verify the database for your generated salt to be unique, you're using a bad salt generator. Make sure to focus on using a good generator instead.
Your hash function also seems weak. Consider using key stretching.

This should not be required:

while(isFound){
   isFound=false;
   String randomSalt=createRandomString(); //creates salt/random string
   ResultSet rs = stmt.executeQuery("SELECT * FROM mvUser where salt="+randomString);
   //check if salt is unique
   while (rs.next()) {
     isFound=true;
   }
}

And for hash, I would expect something like below, where hash is an established hash library (PBKDF2, bcrypt, scrypt, ..).

var hashedPW = hash(iterations, randomSalt, pw);


Depth First Search vs Breadth First Search So after years of teaching myself, I think I'm ready to stand on the shoulders of giants and ask you a good question!
I have a tree of nodes. What I want is a function that returns an array of the entire tree sorted by depth.
In my original attempt I wasn't aware at the time that I was performing a depth first search.
My solution was to:

recursively walk the tree, annotating depth as I go along.
sort the above based on the depth annotation.
filter out the depth annotation and return the sorted array.

That was three steps invoking 3 loops! So then someone alerted me to the concept of a breadth first search. I did my research and built (on my own) a BFS function! It looked so simple and did what I needed.
Then when I timed both versions; completely bafflingly; the cumbersome DFS version is faster! Why???
Here is my depth-first-search:
function dfsElementsInTree(input){
  // perform depth first search but
  // return a depth sorted array of an element or elements and any children
  let output = [];

  if (Symbol.iterator in input)
    // input is a HTMLcollection
    for (const element of input)
      doTraversal(element);
  else
    doTraversal(input);

  return output.sort((a, b) => a.depth - b.depth).map(item=>item.element);

  function doTraversal(element, depth=0) {
    output.push({element, depth});
    if (element.children.length) depth++;
    for (const child of element.children)
      doTraversal(child, depth);
  }
}

Here is my breadth-first-search:
function bfsElementsInTree(input) {
  // perform a breadth first search in order to have elements ordered by depth.
  let output = [];
  let queue = [];
  let visited = [];
  const enqueue = item => {queue.push(item); visited.push(item);};

  if (Symbol.iterator in input)
    // input is a HTMLcollection
    for (const element of input)
      queue.push(element);
  else
    queue.push(input);

  while (queue.length) {
    for (const element of queue)
      for (const child of element.children)
        if (!visited.includes(child))
          enqueue(child);
    output.push(queue.shift());
  }

  return output;
}

Ready for benchmarking here: https://jsben.ch/ZNuAx

But if you want to test it yourself, here's some code to generate some trees:
// Create trees of divs as such:
// (left to right)
// 1
// 1 -> 2
// 1 -> 2 -> 3
// 1 -> 2
// 1

const a1 = document.createElement('div');
const a2 = document.createElement('div');
const a3 = document.createElement('div');
const a4 = document.createElement('div');
const a5 = document.createElement('div');

[a1,a2,a3,a4,a5].forEach(e => e.className ='1');

const b2 = document.createElement('div');
const b3 = document.createElement('div');
const b4 = document.createElement('div');

[b2,b3,b4].forEach(e => e.className ='2');

const c3 = document.createElement('div');

c3.className = '3';

a2.appendChild(b2);

b3.appendChild(c3);
a3.appendChild(b3);

a4.appendChild(b4);

[a1,a2,a3,a4,a5].forEach(e => document.body.appendChild(e));

Thank you so much for your time. It's a real treat to have an expert looking over my shoulder!

I solved it. It turns out my looping logic was a bit out. No need for both a while and a for! (and while we're at it, we don't have to check for visited. Thanks @vnp in comments)

edit: since I'm not actually using a queue, I missed that I don't now need to maintain two arrays! Thanks @Ry in the comments!)

Wrote it again for speed and here it is:

  function bfsElementsInTree(input) {
    // perform a breadth first search in order to have elements ordered by depth. (Deepest last)
    let output = [];

    if (Symbol.iterator in input)
      // input is a HTMLcollection
      for (let i = 0, max = input.length; i < max; i++)
        output[i] = input[i];
    else
      output.push(input);

    for (let i = 0; i < output.length; i++) {
      const children = output[i].children;
      for (let j = 0, max = children.length; j < max; j++)
        output.push(children[j]);
    }

    return output;
  }

And new benchmark: https://jsben.ch/F1zzW


Removing stop words from a Spark Dataframe I am trying to apply a function to two Spark Dataframes (in Zeppelin):

def RemoveStopwords (dfin : dataframe) : Dataframe = {
    val stopwords =

    sc.textFile("/media/ed/Seagate/data/stopwords.txt")

    val tokenizer = new Tokenizer().setInputCol("text").setOutputCol("words")
    val wordsData = tokenizer.transform(dfin)

    // remove stop words
    val remover = new StopWordsRemover().setInputCol("words").setOutputCol("NoStop")
    val dfNoStop= remover.transform(wordsData)

}

and I call it:

Array(dfHive, dfHive1).map(RemoveStopwords)

Is this the right way of doing things? Should I be passing in the stopwords.txt file rather than looking it up twice? Also, is it OK to have : Dataframe (some functions I have seen are declared name : type not just : type)?

In case it helps anybody, I needed to do the following to fix my code:

add import org.apache.spark.sql.DataFrame
Make sure my DataFrame's were capitalized correctly.
add dfNoStop at the end of the function, to stop it returning unit.
make the call val Array(dfNoStop, dfNoStop1)=Array(dfHive, dfHive1).map(RemoveStopwords)


Resizing table widths and making input fields I am trying to simplify my code, which will do several things:

Give the same width to every columns no matter how many columns the table has.
Add input field if there is no text inside a td
Make sure the input field is no wider than its table cell
Make sure, if the input field has size attribute, the input field size will increase based on the size attribute
If the table width is too narrow, increase if necessary

My HTML structure:

          cell 1

          <input size='18'/>

          cell 3

          cell 4

          <input size='15'/>

          <input size='15'/>

jQuery:

$(".test td").each(function(){
        columns = $(this).parent().find('td').length;
        inputField= $(this).find('input')
        tdWidth= $(this).width() - 35;
        inputSize = $(this).find('input').attr('size');
        span = $(this).find('span');
        oldTableWdith = $(this).closest('table').attr('width');

        if($.trim($(span).html()) == '&nbsp;'){
            $(span).html("<input type='text' style='width:" + tdWidth + "px;'></input>");
        }

        inputWidth = inputSize * 11;
        inputField.css('width',inputWidth);

        if(tableWidth < (columns * inputWidth + 50)){
            tableWidth = columns * inputWidth + 50;
        }
        if(tableWidth===0){
            tableWidth = oldTableWdith
        }
        $(this).closest('table').width(tableWidth);
    })

The code works, but I feel like I can greatly reduce it and achieve what I need.
I also need the span tag because I need them to do some other functions. Can anyone help me simplify the code?

At first, you code looks good enough for the set of the requrements you specified. The constructions that you use are simple, so here may be place for the improvements, but this requeres developing understanding in the nontrivial subjects: CSS selectors, jQuery selectors and jQuery chaining details. I think that it may be possible to write down all the code in one chain but it may decrease maintainability.

Some recommendations:

Instead of getting element html use .text().trim().
Use width:100% for inputs to let CSS do the work for you, but apply it only for the inputs without size - use combination of :not and [size] for that.
Tables are stretching, so just remove the table size at the end - it should be autosized to the content.
Use find(...)....end() to chain the queries and separate the realizations of the requirements.
Use static code analysis tools, like Jshint, jslint - it is better to add var to not interfere with the global scope and these tools let know about such kind of the errors and more.
It highly unclear what is the nature of the constants (like 35, 11, etc) in the code. It is better to add at least a very small explanation on them. It greatly improves understanding and review.


Temperature calculator in Rust Just on a chapter 3 of Rust Book so not much good in Rust.
so tried to implement a simple temprature calculator as suggest in this chapter.
so i would like you guys to point anything you feel

use std::io;

fn main() {
    println!("Enter Temperature as 56f or 98c");

    let mut usr_inpt = String::new();

    io::stdin()
        .read_line(&mut usr_inpt)
        .expect("can not read user input");

    let usr_inpt = usr_inpt.trim_end().to_lowercase();

    if usr_inpt.ends_with("f") || usr_inpt.ends_with("c") {
        // remove the last indication 56f -> 56 or 56c -> 56
        let _temp: String = usr_inpt.chars().take(usr_inpt.len() - 1).collect();
        let num: u32 = match _temp.parse() {
            Ok(num) => num,
            Err(_) => 0,
        };

        if usr_inpt.ends_with("f") {
            println!("celcius -> {}", (num - 32) * 5 / 9);
        } else if usr_inpt.ends_with("c") {
            println!("farenheit -> {}", num * 9 / 5 + 32);
        }
    } else {
        println!("invalid input");
    }
}

use std::io;

fn main() {
    println!("Enter Temperature as 56f or 98c");

    let mut usr_inpt = String::new();

    io::stdin()
        .read_line(&mut usr_inpt)
        .expect("can not read user input");

I like that you’re using expect here instead of unwrap.
It allows for a much nicer user experience when things go sideways.
If I was nitpicking, I’d remind you to use proper capitalization and punctuation.

    let usr_inpt = usr_inpt.trim_end().to_lowercase();

    if usr_inpt.ends_with("f") || usr_inpt.ends_with("c") {
        // remove the last indication 56f -> 56 or 56c -> 56
        let _temp: String = usr_inpt.chars().take(usr_inpt.len() - 1).collect();

temp is rarely a good name.
If you meant temperature instead of temporary, it would be good to spell it out.

        let num: u32 = match _temp.parse() {
            Ok(num) => num,
            Err(_) => 0,
        };

Is it really the right thing to return zero in the error case?
I would expect the error case here to be caused by invalid user input (or a bug in your previous parsing logic maybe).
It would probably be best to alert the user to this failure instead of the slightly enigmatic result of 0.

        if usr_inpt.ends_with("f") {
            println!("celcius -> {}", (num - 32) * 5 / 9);
        } else if usr_inpt.ends_with("c") {
            println!("farenheit -> {}", num * 9 / 5 + 32);
        }

It smells a little funny that we’re making the same checks again as we did above. If you extracted the parsing logic into a function, you could just call it for each case. I would probably also extract proper fahrenheit_to_celcius and celcius_to_fahrenheit functions.

    } else {
        println!("invalid input");
    }
}

Error messages should be printed to stderr.
Use the eprintln macro instead.

———————

All in all it’s pretty good for a first go.
I would take a look at using some types for Fahrenheit and Celsius though. Your parse method could return a Temperature that contains either a Celsius or Fahrenheit measurement.

I’ve not run this through the compiler, but hopefully it illustrates the idea.

struct Celsius { value: u32 }
impl Celsius {
    fn to_farhenheit(&self) -> Farhenheit {
        Fahrenheit { value: self.value * 9 / 5 + 32 }
    }
}

struct Fahrenheit { value: u32 }
impl Fahrenheit {
    fn to_celsius(&self) -> Celsius {
        Celsius { value: ( self.value  - 32) * 5 / 9 }
    }
}

enum Temperature {
    Fahrenheit(Fahrenheit),
    Celsius(Celsius)
    Err(String)
}

fn parse_input(input: &str) -> Temperature {
    if input.ends_with("f") {
        Fahrenheit { value: parse_num(input) }
    } else if input.ends_with(“c”) {
        Celsius { value: parse_num(input) }
    } else {
        Err(“Input invalid. Must end with ‘c’ or ‘f’.”)
    }
}

fn parse_num(input: &str) -> u32 {
    let temperature: String = usr_inpt.chars().take(usr_inpt.len() - 1).collect();

    match temperature.parse() {
        Ok(num) => num,
        Err(_) => 0,
    };
}

Then we tie it all together in main.

fn main() {
    println!("Enter Temperature as 56f or 98c");

    let mut usr_inpt = String::new();

    io::stdin()
        .read_line(&mut usr_inpt)
        .expect("can not read user input");

    let temperature = parse_input(usr_inpt.trim_end().to_lowercase());

    match temperature {
        Temperature::Celsius { celsius } => println!("celcius -> {}", celcius.to_fahrenheit()),
        Temperature::Fahrenheit { fahrenheit } => println!(“fahrenheit -> {}”, fahrenheit.to_celsius()),
        Err(reason) => eprintln!(reason)
    }
}

While it’s certainly more code, it raises the level of abstraction in your main function quite a bit.

Greet user
Get input
Parse input
Write out results

This also provides the opportunity (left as an exercise for the reader) to propagate errors from parse_num all the way back up to the user, rather than silently returning an invalid result.


Finding the most frequent character in a string This is in C/C++ (using a c-string as input). I'm curious if my solution could be more efficient than it currently is.

char mostFrequent(char *bytes, int length) {
  char holder[26];
  for (int i = 0; i < 26; i++) {
    holder[i] = 0;
  }

  for (int i = 0; i < length; i++) {
    holder[bytes[i]-97] += 1;
  }

  char b = 97; // a
  int count = holder[0];
  for (int i = 1; i < 26; i++) {
    if (holder[i] > count) {
      count = holder[i];
      b = i+97;
    }
  }

  return b;
}

If you are intended to measure the frequency of bytes, as the name of your input string seems to suggest, rather than just lowercase ASCII letters, then your holder array should be 256 elements long.

This also saves you from all the additions and subtractions of the 'magic' number 97. (Seriously, you could have used 'a'.)

Also, your holder array should consist of ints not chars, because if a certain byte appears more than 256 times in your input string the char will wrap around to zero. Also an array of ints will probably perform more efficiently due to memory alignment.


Python Sum of Pairs Codewars Solution requires optimization Sum of Pairs
Given a list of integers and a single sum value, return the first two values (parse from the left please) in order of appearance that add up to form the sum.
sum_pairs([11, 3, 7, 5],         10)
#              ^--^      3 + 7 = 10
== [3, 7]

sum_pairs([4, 3, 2, 3, 4],         6)
#          ^-----^         4 + 2 = 6, indices: 0, 2 *
#             ^-----^      3 + 3 = 6, indices: 1, 3
#                ^-----^   2 + 4 = 6, indices: 2, 4
#  * entire pair is earlier, and therefore is the correct answer
== [4, 2]

sum_pairs([0, 0, -2, 3], 2)
#  there are no pairs of values that can be added to produce 2.
== None/nil/undefined (Based on the language)

sum_pairs([10, 5, 2, 3, 7, 5],         10)
#              ^-----------^   5 + 5 = 10, indices: 1, 5
#                    ^--^      3 + 7 = 10, indices: 3, 4 *
#  * entire pair is earlier, and therefore is the correct answer
== [3, 7]

Negative numbers and duplicate numbers can and will appear.

NOTE: There will also be lists tested of lengths upwards of 10,000,000 elements. Be sure your code doesn't time out.

def sum_pairs(ints, s):
    '''
    Params:
        ints: list[int]
        s: int --- sum
    Return
        First two values from left in ints that add up to form sum
    '''
    ints_dict = {k:v for v,k in enumerate(ints)} # O(n)
    earliest_ending_idx = float('inf')
    earliest_pair = None
    for idx, num in enumerate(ints): # O(n)
        if idx >= earliest_ending_idx:
            return earliest_pair
        try:
            second_num_idx = ints_dict[s-num]
            if second_num_idx != idx:
                if max(second_num_idx, idx) < earliest_ending_idx:
                    earliest_ending_idx = max(second_num_idx, idx)
                    earliest_pair = [num, s-num]
        except KeyError:
            # s-num not in dict
            pass
    return earliest_pair

It seems to me that my code is O(n) in time complexity but I can't submit the solution since it took too long to run. Am I mistaken in my analysis and is there a better way to solve this problem?

your solution first maps all of the numbers, and then check for pairs.
what if, for instance, the list of numbers is very long, and the first two items already sum up to the desired sum?
I think that the optimal solution would iterate over the list only once.


Cleaning and extracting meaningful text from tweets I have a dataset of around 200,000 tweets. I am running a classification task on them. Dataset has two columns - class label and the tweet text. In the preprocessing step I am passing the dataset through following cleaning step:

import re
from nltk.corpus import stopwords
import pandas as pd

def preprocess(raw_text):

    # keep only words
    letters_only_text = re.sub("[^a-zA-Z]", " ", raw_text)

    # convert to lower case and split
    words = letters_only_text.lower().split()

    # remove stopwords
    stopword_set = set(stopwords.words("english"))
    meaningful_words = [w for w in words if w not in stopword_set]

    # join the cleaned words in a list
    cleaned_word_list = " ".join(meaningful_words)

    return cleaned_word_list

def process_data(dataset):
    tweets_df = pd.read_csv(dataset,delimiter='|',header=None)

    num_tweets = tweets_df.shape[0]
    print("Total tweets: " + str(num_tweets))

    cleaned_tweets = []
    print("Beginning processing of tweets at: " + str(datetime.now()))

    for i in range(num_tweets):
        cleaned_tweet = preprocess(tweets_df.iloc[i][1])
        cleaned_tweets.append(cleaned_tweet)
        if(i % 10000 == 0):
            print(str(i) + " tweets processed")

    print("Finished processing of tweets at: " + str(datetime.now()))
    return cleaned_tweets

cleaned_data = process_data("tweets.csv)

And here is the relevant output:

Total tweets: 216041
Beginning processing of tweets at: 2017-05-16 13:45:47.183113
Finished processing of tweets at: 2017-05-16 13:47:01.436338

It's taking approx. 2 minutes to process the tweets. Although it looks relatively a small timeframe for current dataset I would like to improve it further especially when I use a dataset of much bigger size.

Can the steps/code in the preprocess(raw_text) method be improved in order to achieve faster execution?

Copying my answer from SO:

You can use pandas vectorized string methods to do your processing and it also removes the for loop for more efficient pandas operations, this should give you some speed.

# column you are working on
df_ = tweets_df[1]

stopword_set = set(stopwords.words("english"))

# convert to lower case and split
df_ = df_.str.lower().split()

# remove stopwords
df_ = df_.apply(lambda x: [item for item in x if item not in stopword_set])

# keep only words
regex_pat = re.compile(r'[^a-zA-Z\s]', flags=re.IGNORECASE)
df_ = df_.str.replace(regex_pat, '')

# join the cleaned words in a list
df_.str.join("")

Also I've changed your regex to [^a-zA-Z\s] so that it does not match the space character.


Let's go through it step-by-step.

for (int j = i, k = 0; j < limit; j++, k++) {
    x[k] = Integer.parseInt(String.valueOf(largeNum.charAt(j)));

    sum = x[k] * sum;

}

In this for loop, whose goal is to calculate the product of the limit digits starting at the index i, there are multiple concerns:

You are converting the character at the index j to an integer by first turning it into a String and parsing that String into an integer. You can do it in a more direct manner by using Character.getNumericValue(char); this will directly return the numeric value of the character, i.e. 5 for the character '5'. Also, since we know before-hand that we will be dealing with digits that are only [0-9] (and not all Unicode digits), we can even use a simple arithmetic calculation with largeNum.charAt(j) - '0'.
You are using a pre-allocated array to store this integer. You don't need it: you can just store the integer as a local variable. This will also simplify the for loop conditions: it is currently iterating over both a j and a k when only one is really needed.

With those considerations, you could have:

for (int j = i; j < limit; j++) {
    int digit = largeNum.charAt(j) - '0';
    sum *= digit;  // which is the same as "sum = digit * sum"
}

if (sum < previousSum) {
    sum = previousSum;

}
previousSum = sum;

This works but is awkward to read: we're interested in getting the maximum sum and this code is dealing with a "sum" and a "previous sum". Also, why are talking about "sum" when the problem deals with "products"?

Consider renaming the result to maximumProduct and the current product to product. With this, you can have:

maximumProduct = Math.max(maximumProduct, product);

which is a lot clearer: at the end of calculating the product for the current index, we retrieve the maximum of the current maximum and the calculated product, using Math.max.

sum = 1;

The first remark is: what is this code doing at the end of the for loop? It actually initializes a value for sum that will be used for the product of the next index. Thus, this should be placed at the top of the loop: let the current index initialize its temporary values correctly.

if (limit > largeNum.length()) {
    break;
}

It took me a bit of time to figure out why this code was there. Its purpose is actually to "correct" the fact that the subsequent loop considers limit digits because its ending condition is

//                  v---v
for (int j = i; j < limit; j++) {

You should not change the value of limit. This in fact a constant of the problem: it is the number of consecutive digits to consider. Thus, what you need to do is:

At index i, consider the next limit digits with

for (int j = i; j < i + limit; j++)
//                  ^-------^

End the main loop when there are less than limit digits left with

for (int i = 0; i < largeNum.length() - limit; i++)
//                  ^-----------------------^

Additional comments

As said before, limit is a constant of this problem. At the very least, it should be given as parameter of computeProduct, just like largeNum. Both of those could then be made constants (i.e. private static final).
computeProduct, despite its name, does more than computing the product; it actually prints it. Consider changing the method so that it returns the wanted product. Then, you can have the main method print it.

All in all, this is what you end up with:

private long computeProduct(int limit, String largeNum) {
    long maximumProduct = 1;
    for (int i = 0; i < largeNum.length() - limit; i++) {
        long product = 1;
        for (int j = i; j < i + limit; j++) {
            int digit = largeNum.charAt(j) - '0';
            product *= digit;
        }
        maximumProduct = Math.max(maximumProduct, product);
    }
    return maximumProduct;
}


Searching based on a query string and returning JSON ActionResult Using a Select2, what's the best way to refactor multiple if statements.

This code returns Json to a select2 based on what information it gets in the query. The action result is used in many places so it has accommodate different parameter being passed.

public ActionResult *********(string query = "", int? id = null, int? accountId = null)
    {
        var empty = new object();

        if (query == "" && accountId == null)
        {
            return Json(empty, JsonRequestBehavior.AllowGet);
        }

        if (query == "" && accountId != null)
        {
            var search = Db.Entities.Find(accountId);
            var suggestions = Db.Entities.Where(
                x => x.Name.Contains(search.Name) && x.EntityId != accountId && !String.IsNullOrEmpty(x.Group.Label))
                .ToList()
                .Select(item => new { id = item.GroupId, text = item.Group.Label });

            return Json(suggestions, JsonRequestBehavior.AllowGet);

        }

        if (id != null)
        {
            var accountGroup = Db.Groups.Find(id);
            if (accountGroup != null)
                return Json(new { id = accountGroup.GroupId, text = accountGroup.Label },
                    JsonRequestBehavior.AllowGet);
            return Json(null, JsonRequestBehavior.AllowGet);
        }
        var list = Db.Groups.Where(
            r => !String.IsNullOrEmpty(query) && query.Length > 2 && (r.StatusId == GroupStatus.Both || r.StatusId == GroupStatus.Customer) && r.Label.ToLower().Contains(query.ToLower())).OrderBy(r => r.Label).Select(item => new { id = item.GroupId, text = item.Label }).ToList();
        if (!list.Any())
        {
            return Json(new List<object>() { new { id = query, text = query } }, JsonRequestBehavior.AllowGet);
        }
        return Json(list, JsonRequestBehavior.AllowGet);
    }

In following query

var suggestions = Db.Entities.Where(x => x.Name.Contains(search.Name)
                              && x.EntityId != accountId
                              && !String.IsNullOrEmpty(x.Group.Label))
                 .ToList()
                 .Select(item => new { id = item.GroupId, text = item.Group.Label });

You shouldn't call ToList(). Read this answer from a question on StackOverflow. This is the essential part:

  For example, in an Entity Framework query we can only use a restricted number of methods. So if, for example, we need to use one of our own methods in a query we would typically write something like

   var query = context.Observations.Select(o => o.Id)
               .AsEnumerable().Select(x => MySuperSmartMethod(x))

  ToList – which converts an IEnumerable to a List – is often used for this purpose as well. The advantage of using AsEnumerable vs. ToList is that AsEnumerable does not execute the query. AsEnumerable preserves deferred execution and does not build an often useless intermediate list.

  On the other hand, when forced execution of a LINQ query is desired, ToList can be a way to do that.

No need to check twice for the query to see if it is empty. Your code would be better like this:

if (query == "")
{
    if (accountId == null)
    {
        //return Json
    }
    else
    {
        //return some other Json
    }
}

You can improve the performance of your LinQ query by building it correctly. First check for the EntityId != accountId, then !String.IsNullOrEmpty(x.Group.Label) and last for x.Name.Contains(search.Name):

var suggestions = Db.Entities.Where(x => x.EntityId != accountId
                              && !String.IsNullOrEmpty(x.Group.Label)
                              && x.Name.Contains(search.Name))
                 .AsEnumerable()
                 .Select(item => new { id = item.GroupId, text = item.Group.Label });

Notice that I changed ToList to AsEnumerable.

Variable names like list and search are not meaningful. Give clear names to your variables that kind of represent what they are holding.

var list = Db.Groups.Where(...);

would become:

var groups = Db.Groups.Where(...);


Project Euler #16 - Sum of all digits of 2^1000 2^15 = 32768 and the sum of its digits is 3 + 2 + 7 + 6 + 8 = 26.

  What is the sum of the digits of the number 2^1000?

    BigInteger big = new BigInteger("2");
    big = big.pow(1000);
    String num = big.toString();
    System.out.println(num);
    int result = 0;
    for(char i : num.toCharArray()) {
        result += Integer.parseInt(String.valueOf(i));
    }
    System.out.println(result);

This looks good. I assume this results in correct answer.

What you can use instead of converting to String and back to int is to use divideAndRemainder method with 10 since we need to treat this as a base 10 number.
This method is available in BigInteger for situations like this.
We can also directly use BigInteger constants such as TWO and TEN.

Alternative Implementation

BigInteger big = BigInteger.TWO.pow(1000);
String num = big.toString();
System.out.println(num);

int result = 0;
BigInteger[] components;

components = big.divideAndRemainder(BigInteger.TEN);
while (components[0].signum() != 0) {
    result += components[1].intValue();
    components = components[0].divideAndRemainder(BigInteger.TEN);
}
result += components[1].intValue();
System.out.println(result);

I've used signum method here to check if result after integer division is zero.
Note: This seems to be creating lot of objects.

Benchmark with JMH

After the some discussion in comments with @TorbenPutkonen, I agreed with TorbenPutkonen that alternative implementation might be creating more objects. However there is no way to see which implementation performs faster without doing a benchmark.

public class X {

    public static void main(String[] a) throws Exception {
        org.openjdk.jmh.Main.main(a);
    }

    @State(Scope.Benchmark)
    public static class BenchmarkState {
        BigInteger multiple =  BigInteger.TWO.pow(1000);
        public BenchmarkState() {
            System.out.println(multiple);
        }
    }

    @Benchmark
    @Warmup(iterations = 5)
    public int withDivide(BenchmarkState x) {
        BigInteger[] components;
        components = x.multiple.divideAndRemainder(BigInteger.TEN);
        int result = 0;
        while (components[0].signum() != 0) {
            result += components[1].intValue();
            components = components[0].divideAndRemainder(BigInteger.TEN);
        }
        result += components[1].intValue();
        return result;
    }

    @Benchmark
    @Warmup(iterations = 5)
    public int withChars(BenchmarkState x) {
        String num = x.multiple.toString();
        int result = 0;
        for(char i : num.toCharArray()) {
            result += Integer.parseInt(String.valueOf(i));
        }
        return result;
    }

    @Benchmark
    @Warmup(iterations = 5)
    public int withCharsNumerical(BenchmarkState x) {
        String num = x.multiple.toString();
        int result = 0;
        for(char i : num.toCharArray()) {
            result += Character.getNumericValue(i);
        }
        return result;
    }

    @Benchmark
    @Warmup(iterations = 5)
    public int withCharAt(BenchmarkState x) {
        String num = x.multiple.toString();
        int len = num.length();
        int result = 0;
        for(int i = 0; i < len; i++) {
            result += Integer.parseInt(String.valueOf(num.charAt(i)));
        }
        return result;
    }

    @Benchmark
    @Warmup(iterations = 5)
    public int withCharsNumericalCharAt(BenchmarkState x) {
        String num = x.multiple.toString();
        int len = num.length();
        int result = 0;
        for(int i = 0; i < len; i++) {
            result += Character.getNumericValue(num.charAt(i));
        }
        return result;
    }
}

# Run complete. Total time: 00:21:29

Benchmark                    Mode  Cnt       Score      Error  Units
X.withCharAt                thrpt  200  117285.320 ±  644.505  ops/s
X.withChars                 thrpt  200  116882.706 ±  779.233  ops/s
X.withCharsNumerical        thrpt  200  110849.659 ± 3901.095  ops/s
X.withCharsNumericalCharAt  thrpt  200  121480.705 ± 2040.597  ops/s
X.withDivide                thrpt  200   11306.787 ±   35.711  ops/s

This concludes that original version is roughly 10x faster than divideAndRemainder
Original version is also slightly faster than using getNumericValue by itself.
However we can use charAt and avoid creating a character array too.

Why is using divideAndRemainder slow?

toString method of BigInteger uses a faster algorithm to create the string representation.
divideAndRemainder creates lot of BigInteger objects.


Closest element in std map when discarding values I have the following situation: I have two std::map<int,float> and I want to go through the first map one by one and find the element in the second map with the key closest to the key in the first map. Now, this plain situation is obviously addressed in other question like e.g. this one, but here is where my situation deviates:
There might be elements in the second map that I do not want to consider because the value is below some criterion and thus even if the element is the closest, it will be discarded and now I have to check two other elements against the key. Below is a bit of dummy code that illustrates the input data and also a bit of code that seems to achieve what I want, but I am not sure if there is maybe a better/cleaner way of doing this.

#include <iostream>
#include <map>
#include <cmath>

void setClosest( std::map<int,float>::iterator & it, std::map<int,float>::iterator &upper, std::map<int,float>::iterator &lower, int key ) {
  it = fabs( upper->first - key ) < fabs( lower->first - key ) ? upper : lower;
}

int main(int, char**)
{
  std::map<int, float> map1;
  std::map<int, float> map2;

  map1[5000]  = 5.5;
  map1[10000] = 5.2;
  map1[12000] = 5.7;
  map1[15000] = 5.3;
  map1[20000] = 5.5;
  map1[25000] = 5.9;

  map2[10400] = 523.0;
  map2[12500] = 99.0;
  map2[15200] = 67.0;
  map2[15400] = 76.0;
  map2[20200] = 511.0;
  map2[25800] = 567.0;

  std::map<int,float>::iterator it = map1.begin();
  std::map<int,float>::iterator lower;
  std::map<int,float>::iterator upper;
  std::map<int,float>::iterator closest;

  for( ; it != map1.end(); ++it ) {
    int key     = it->first;
    float value = it->second;

    lower = map2.lower_bound(key);
    upper = ( lower == map2.begin() ? lower : lower-- );

    setClosest( closest, upper, lower, key);

    std::cout << key << ' ' << upper->first << ' ' << lower->first << " -> " << closest->first << std::endl;

    while( closest->second < 100 ) {
      std::cout << "skipping too low value " << closest->first << ' ' << closest->second << std::endl;
      closest == upper ? upper++ : lower--;
      setClosest( closest, upper, lower, key);
    }

    std::cout << key << ' ' << upper->first << ' ' << lower->first << " -> " << closest->first << std::endl;

  }

  return 0;
}

Output

5000 10400 10400 -> 10400
5000 10400 10400 -> 10400
10000 10400 10400 -> 10400
10000 10400 10400 -> 10400
12000 12500 10400 -> 12500
skipping too low value 12500 99
12000 15200 10400 -> 10400
15000 15200 12500 -> 15200
skipping too low value 15200 67
skipping too low value 15400 76
skipping too low value 12500 99
15000 20200 10400 -> 10400
20000 20200 15400 -> 20200
20000 20200 15400 -> 20200
25000 25800 20200 -> 25800
25000 25800 20200 -> 25800

Consider replacing the while loop with two for loops that increment lower and upper iterators till they have a suitable value - and only afterwards setting closest - it might be doing more operations but it is more cache friendly.

Or simply create a new map2 with only valid values (simply filtrate unwanted).

Though, different situation have different preferred solutions - there is a trade-off which solution is better depending on number of undesirable values in map2 and relative sizes of map1 and map2.

I don't think that you can improve it beyond that, lest you have some extra knowledge you haven't shared. In certain cases, you might want to implement some completely different solutions that are not reliant on map.

About the code itself, there are just a few minor issues:

You don't take care of the case when lower_bound return an end, in this case program will crash or UB. Same problem inside the while scope.
There is no need to declare the four iterators outside the scope. Just declare them with auto when you instantiate them. Nobody cares to see std::map<int,float>::iterator.
replace setClosest with

  template<typename MapItr>
  auto getClosestMapIterator(const MapItr &upper, const MapItr & lower, int key ) -> const MapItr&
  {
     return abs( upper->first - key ) < abs( lower->first - key ) ? upper : lower;
  }

and adjust usage auto closest = getClosestMapIterator(upper, lower, key); Also you better use abs instead of fabs, as fabs is for floating point types, so you unnecessarily cast integers to double with fabs.
Inside while the line closest == upper ? upper++ : lower--; is confusing. Simply write if scope. Don't use the expression cond ? ret_on_true : ret_on_false for side-effects, only when you want to get the return value.


Dynamically-sized stack - follow-up 2 Follow up of - Dynamically-sized stack - follow up

I've took the tips given to me, and what I did now is:

changing the function names to have a prefix
making the pop() function only popping and not returning the value as well
checking for OOM exception / scenario that 0 is passed to capacityIncrement variable when initializing the stack (in those cases, if trying to push values to the stack, just ignore)
added a decrement mechanism, when popping values a check will be made to see if there are (capacityIncrement / 2 + capacityIncrement) elements in the stack that are not in use, and if so, decrement the stack size by capacityIncrement

#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>

typedef struct
{
    int *elementData;
    unsigned int stackSize;
    unsigned int capacityIncrement;
    unsigned int elementCount;
} Stack;

void stack_initialize(Stack*, unsigned int);
void stack_push(Stack*, int);
void stack_pop(Stack*);
int stack_peek(const Stack*);
void stack_destroy(Stack*);
bool stack_isEmpty(const Stack*);
void stack_setCapacityIncrement(Stack*, unsigned int);
unsigned int stack_getCapacityIncrement(const Stack*);
unsigned int stack_getNumberOfElements(const Stack*);
unsigned int stack_getSize(const Stack*);

int main()
{
    Stack s1;
    stack_initialize(&s1, 4);
    for(int i = 0; i < 25; i++)
    {
        stack_push(&s1, i + 1);
    }
    stack_pop(&s1);
    stack_push(&s1, 88);
    stack_push(&s1, 25);
    printf("The top of the stack is %d.\n", stack_peek(&s1));
    while(!stack_isEmpty(&s1))
    {
        int top = stack_peek(&s1);
        stack_pop(&s1);
        printf("Popping %d from the top of the stack.\n", top);
        printf("Size of stack is %d.\n", s1.stackSize);
    }
    return 0;
}

void stack_initialize(Stack *p, unsigned int capacityIncrement)
{
    p->elementData = NULL;
    p->stackSize = 0;
    p->capacityIncrement = capacityIncrement;
    p->elementCount = 0;
}

void stack_push(Stack *p, int value)
{
    if (p->elementCount == p->stackSize)
    {
        int newStackSize = p->stackSize + p->capacityIncrement;
        void *temp = realloc(p->elementData, sizeof(*p->elementData) * newStackSize);
        if (temp == NULL || newStackSize == 0)
        {
            return;
        }
        p->stackSize = newStackSize;
        p->elementData = temp;
    }
    p->elementData[p->elementCount] = value;
    p->elementCount++;
}

void stack_pop(Stack *p)
{
    if (!stack_isEmpty(p))
    {
        p->elementCount--;
        if(p->stackSize - p->elementCount == p->capacityIncrement / 2 + p->capacityIncrement)
        {
            int newStackSize = p->stackSize - p->capacityIncrement;
            p->elementData = realloc(p->elementData, sizeof(*p->elementData) * newStackSize);
            p->stackSize = newStackSize;
        }
    }
}

int stack_peek(const Stack *p)
{
    if (!stack_isEmpty(p))
    {
        return p->elementData[p->elementCount - 1];
    }
    return 0;
}

void stack_destroy(Stack *p)
{
    free(p);
}

bool stack_isEmpty(const Stack *p)
{
    return p->elementCount == 0;
}

void stack_setCapacityIncrement(Stack *p, unsigned int capacityIncrement)
{
    p->capacityIncrement = capacityIncrement;
}

unsigned int stack_getCapacityIncrement(const Stack *p)
{
    return p->capacityIncrement;
}

unsigned int stack_getNumberOfElements(const Stack *p)
{
    return p->elementCount;
}

unsigned int stack_getSize(const Stack *p)
{
    return p->stackSize;
}

Use same case prefix for type "Stack" and functions "stack" --> "gStack" or whatever.
Use correct print specifier.  This implies that either OP does not have all warnings enabled or is using a weak compiler.  Suggest remedying that.

// printf("Size of stack is %d.\n", s1.stackSize);
printf("Size of stack is %u.\n", s1.stackSize);

Use consistent sign-ness

// int newStackSize;
unsigned newStackSize;

In push(), newStackSize test should be corrected or eliminated.

// if (temp == NULL || newStackSize == 0)

if (temp == NULL && newStackSize > 0)

// or if confident newStackSize will never be 0 here
if (temp == NULL)

Silently failing push() due to OOM is a concern.
pop() should do similar OOM protection as pop().  Even though it sounds silly that reducing memory usage should ever fail.
Consider putting all memory allocation into one helper function.  (BTW: destroy() did not free p->elementData.

static int stack_realloc(int **ptr, unsigned *oldsize, unsigned newsize) {
  if (newsize > 0) {
    void *newptr = realloc(*ptr, newsize);
    if (newptr == NULL) return 1; // fail
    *ptr = newptr;
  }
  else {
    free(*ptr);
    *ptr = NULL;
  }
  *oldsize = newsize;
  return 0; // success;
}

// push()
unsigned newStackSize = p->stackSize + p->capacityIncrement;
if (stack_realloc(&p->elementData, &p->stackSize, newStackSize);

// pop()
// change from == to >=
if(p->stackSize - p->elementCount >= p->capacityIncrement / 2 + p->capacityIncrement) {
  unsigned newStackSize = p->stackSize - p->capacityIncrement;
  // No problem is realloc failed, just continue with current stack
  stack_realloc(&p->elementData, &p->stackSize, newStackSize);

// destroy()
stack_realloc(&p->elementData, &p->stackSize, 0);
// Do not free `p`.
// free(p);


Automate the Boring Stuff – Chess Dictionary Validator – Optimal Solution Brief introduction: so the point of the question is that the author doesn't really talk about good or bad code quality and I really wanted some feedback to have a grasp of how the code for others to read should look like. I mean, for the very first solution of the task I would check different parameters manually just to finish it and the function looked really bulky, so I took some time to make it neat.
Task: the dictionary value represent a chess board, for example {'1h': 'bking', '6c': 'wqueen', '2g': 'bbishop', '5h': 'bqueen', '3e': 'wking'}. Write a function named that takes a dictionary argument and returns True or False depending on if the board is valid.
A valid board will have exactly one black king and exactly one white king. Each player can only have at most 16 pieces, at most 8 pawns, and all pieces must be on a valid space from '1a' to '8h'; that is, a piece can’t be on space '9z'. The piece names begin with either a 'w' or 'b' to represent white or black, followed by 'pawn', 'knight', 'bishop', 'rook', 'queen', or 'king'. This function should detect when a bug has resulted in an improper chess board.
My solution:
def chessboardCheck(board):

    piecesDict={'pawn':8,'knight':2,'bishop':2,'rook':2,'queen':1,'king':1} #to count pieces on the board

    if list(board.values()).count('wking')!=1 or list(board.values()).count('bking')!=1: #kings check
        print('The board is invalid: kings requirement not met.')
        return False
    for piece in board.values():
        if piece[0] not in 'wb': #piece color
            print('The board is invalid: improper color reference.')
            return False
        elif piece[1:] not in piecesDict.keys(): #piece type
            print('The board is invalid: improper piece reference.')
            return False
        elif list(board.values()).count(piece) > piecesDict.get(piece[1:]): #pieces count
            print('The board is invalid: improper pieces count.')
            return False
    for cell in board: #axis correctness
        if cell[0] not in '12345678' or cell[1] not in 'abcdefgh':
            print('The board is invalid: improper cell reference.')
            return False

    print('All checks are clear: the board is valid.')
    return True

This is the least lines I could come up with, but if there are more shortcuts to take, please do not hesitate to point! Also I would like to hear opinions on introducing variables: I tried to avoid them wherever I knew how, but maybe it's not always the way to go? Thanks!

As you may know already, your validation rules are both incomplete and too strict. For example, a black bishop must be on a black square, and there can be more-than-expected numbers of the pieces after a pawn is advanced and promoted (usually, but not always, to a queen). I'll ignore those issues and focus on the code.
Get the so-called magic strings and numbers out of the code and into named
constants and/or data structures. Here's a draft of some constants that might
be handy for your task. As you add more validation checks, you might need to
augment or adjust these.
import sys
from collections import namedtuple

BLACK = 'b'
WHITE = 'w'

KING = 'king'
QUEEN = 'queen'
ROOK = 'rook'
BISHOP = 'bishop'
KNIGHT = 'knight'
PAWN = 'pawn'

COLORS = {BLACK, WHITE}
PIECES = {KING, QUEEN, ROOK, BISHOP, KNIGHT, PAWN}

RANKS = set('12345678')
COLUMNS = set('abcdefgh')

VALID_COUNTS = {
    PAWN:   range(0, 9),
    KNIGHT: range(0, 3),
    BISHOP: range(0, 3),
    ROOK:   range(0, 3),
    QUEEN:  range(0, 2),
    KNIGHT: range(1, 2),
}

Set up a simple data structure to facilitate testing and debugging
as you write the script. For example, we have the board you gave us
and an invalid board I added.
INPUT_BOARDS = {
    'orig': {
        '1h': 'bking',
        '6c': 'wqueen',
        '2g': 'bbishop',
        '5h': 'bqueen',
        '3e': 'wking',
    },
    'bad1': {
        '9h': 'bking',
        '9x': 'wking',
    },
}

The input format for a board is not convenient for validation because it
glues together rank-plus-column and color-plus-piece. Do parsing first, validation
second. There are various ways to arrange the parsing, but an easy, low-tech
way is with a namedtuple -- an immutable container that behaves like a tuple
but also allows one to access the attributes via their names.
By parsing the input board immediately, you can simplify the rest of the code.
The validation function should take and return data (e.g. the first
error or, even better, all errors). It should not print.
Print only in the simple, outer-shell of the program (e.g. a main() function), not it
its more complex, algorithmic center (where you are doing validation).
ParsedCell = namedtuple('ParsedCell', 'cell color_piece rank column color piece')

def main(args):
    board = parse_input_board(INPUT_BOARDS[args[0]])
    errors = check_board(board)
    if errors:
        for e in errors:
            print(e)
    else:
        print('OK')

def parse_input_board(input_board):
    return tuple(
        ParsedCell(
            cell,
            color_piece,
            cell[0:1],       # A safe technique even if cell is an empty string.
            cell[1:],
            color_piece[0:1],
            color_piece[1:],
        )
        for cell, color_piece in input_board.items()
    )

Now sitting on a more solid foundation, the program's validation
code becomes (1) simpler to write and (2) easier to read
because it is more declarative or self-documenting.
def check_board(board):
    errors = []
    for pcell in board:
        if pcell.rank not in RANKS:
            msg = emsg('Invalid rank', pcell.cell)
            errors.append(msg)

        if pcell.color not in COLORS:
            msg = emsg('Invalid color', pcell.cell)
            errors.append(msg)

        # Etc.

    return errors

def emsg(msg, item):
    return f'{msg}: {item}'

if __name__ == '__main__':
    main(sys.argv[1:])


Transposing a 2D array Would you have any suggestions for improvements for the code below?

def my_transpose(arr)
  # number of rows
  m = arr.count

  #number of columns
  n = arr[0].count

  transposed_arr = Array.new(n) { Array.new(m) }

  # loop through the rows
  arr.each_with_index do |row, index1|

    # loop through the colons of one row
    row.each_with_index do |num, index2|

      # swap indexes to transpose the initial array
      transposed_arr[index2][index1] = num
      p transposed_arr
    end
  end
  transposed_arr
end

As already mentioned, Ruby's Array comes with a built-in #transpose method.

But, for the purposes of an exercise, there are multiple ways to do the same thing.

For instance, one way (which only works for square matrices!) is to use #map:

def my_transpose(arr)
  arr.each_with_index.map do |_, index|
    arr.map { |row| row[index] }
  end
end

Or you can use reduce (this will work for rectangular matrices):

def my_transpose(arr)
  arr.reduce([]) do |transposed, row|
    row.each_with_index do |value, col|
      transposed[col] ||= []
      transposed[col] << value
    end
    transposed
  end
end

Or do basially the same thing, just using each_with_object:

def my_transpose(arr)
  arr.each_with_object([]) do |row, transposed|
    row.each_with_index do |value, col|
      transposed[col] ||= []
      transposed[col] << value
    end
  end
end

But keeping more of your code, you could use Array#new as your loop, since it accepts a block.

For instance:

def my_transpose(arr)
  rows = arr.count
  cols = arr.first.count
  Array.new(cols) do |col|
    Array.new(rows) do |row|
      arr[row][col]
    end
  end
end

Of course, you'll want to check a few things. Firstly, whether there are any rows in the matrix at all (if not, the arr[0].count will fail), and whether the matrix is actually rectangular:

def my_transpose(arr)
  return [] if arr.empty?

  rows = arr.count
  cols = arr.first.count

  raise ArgumentError unless arr.all? { |row| row.count == cols }

  Array.new(cols) do |col|
    Array.new(rows) do |row|
      arr[row][col]
    end
  end
end

There are yet more ways to do things, but again: It's already built in as arr.transpose.


Refactor a selection validator I have a data table and have to validate every field in it. I have refactor this code to this below, but the complexity is 15(!!)  Should I make something like dictionary with type as Key and Func as Value?

private bool CheckField(DataRow dataRow, ValidationField validationField)
        {
            bool result = false;
            if (validationField.Requiered)
            {
                if (validationField.Type == typeof (int))
                {
                    result = this.CheckIntegerAndNotNull(dataRow[validationField.Name].ToString());
                }
                else if (validationField.Type == typeof (DateTime))
                {
                    result = this.CheckDateTimeAndNotNull(dataRow[validationField.Name].ToString());
                }
            }
            else
            {
                if (validationField.Type == typeof (int))
                {
                    result = this.CheckIntegerOrNull(dataRow[validationField.Name].ToString());
                }
                else if (validationField.Type == typeof(DateTime))
                {
                    result = this.CheckDateTimeOrNull(dataRow[validationField.Name].ToString());
                }
                else if (validationField.Type == typeof(string))
                {
                    result = this.CheckStringOrNull(dataRow[validationField.Name].ToString(),
                        validationField.MaxLength.Value);
                }
                else if (validationField.Type == typeof(decimal))
                {
                    result = this.CheckDecimalOrNull(dataRow[validationField.Name].ToString());
                }
            }
            return result;
        }

 public class ValidationField
    {
        public Type Type { get; set; }
        public string Name { get; set; }
        public bool Requiered  { get; set; }
        public int? MaxLength { get; set; }
    }

The first simplification is for the Required check. All you need to do here is check that the value is not null, you don't care about the type. Then you can do the type-specific check later. So remove the outermost if...else and simply have:

if (validationField.Requiered && DBNull.Value.Equals(validationField.Name]))
    return false;

The next section is a bit more difficult. However, one approach would be something like:

var dataString = dataRow[validationField.Name].ToString();
return dataString == Convert.ChangeType(dataString, validationField.Type).ToString();

What we're doing here is using .NET's Convert.ChangeType method to change from the string to your desired primative type. Then, if the string representation of the converted item exactly matches the original string, we say that the string must exactly represent the converted item, meaning it is of the correct data type. You may want to also pass in an IFormatProvider for additional safety.

Putting those two code blocks together would pretty much give you your full method. You may need one additional check for the max length of a string though.

Other than the above alternatives, your code mostly looks good. One thing I'd do is scrap the result variable, and return directly from inside your if statements. The this. prefixes are also unnecessary for PascalCased method names.

As a more general design note, it might be preferable to do this validation at a different point in your process. For example, DataColumns can be set to require a particular Type, and can also be told whether or not to accept null values using the AllowDBNull property, meaning the table will largely do this validation for you.