Mini HttpClient json post by Action parameter This is an implementation of HttpClient Json Post by Action parameter.

Logic:

  It is mainly convenient to pass the url and object (automatically converted to json by Json.Net), and can use Action as a parameter to customize function.
  default timeout is 15 sec , if you want to change the time , it could change timeout parameter value.

Code:

using System;
using System.Net.Http;
using System.Text;
using System.Threading.Tasks;
using Newtonsoft.Json;
public class Program
{
    public static void Main(string[] args)
    {
        Test().Wait();
    }

    public static async Task Test()
    {
        //SuccessExecute
        await Execute("https://jsonplaceholder.typicode.com/posts", new { value = "ITWeiHan" }); //Result : {"value": "ITWeiHan","id": 101}
        //ErrorExecute
        await Execute("https://jsonplaceholder.typicode.com/error404", new { value = "ITWeiHan" }); //Result : Error 404
    }

    public static async Task Execute(string url, object reqeustBody)
    {
        await HttpClientHelper.PostByJsonContentTypeAsync(url, reqeustBody
            , successFunction: responsebody =>
            {
                //Your Success Logic
                Console.WriteLine("Success");
                Console.WriteLine(responsebody);
            }, errorFunction: httpRequestException =>
            {
                //Your Error Solution Logic
                Console.WriteLine("Error");
                Console.WriteLine(httpRequestException.Message);
            }
        );
    }
}

public static class HttpClientHelper
{
    public static async Task PostByJsonContentTypeAsync(string url, object reqeustBody, Action<string> successFunction, Action<HttpRequestException> errorFunction, int timeout = 15)
    {
        var json = JsonConvert.SerializeObject(reqeustBody);

        using (var client = new HttpClient() { Timeout = TimeSpan.FromSeconds(timeout) })
        using (var request = new HttpRequestMessage(HttpMethod.Post, url))
        using (var stringContent = new StringContent(json, Encoding.UTF8, "application/json"))
        {
            request.Content = stringContent;
            try
            {
                using (var httpResponseMessage = await client.SendAsync(request))
                {
                    HttpResponseMessage response = await client.GetAsync(url);
                    response.EnsureSuccessStatusCode();

                    var responseBody = await httpResponseMessage.Content.ReadAsStringAsync();
                    successFunction(responseBody);
                }
            }
            catch (HttpRequestException e)
            {
                errorFunction(e);
            }
        }
    }
}

Great job. Here are some minor notes though.

In newer versions of C# you can leverage async Main in order not to wait for your test method.

public static async Task Main(string[] args)
{
    await Test();
}

You can speed things up a bit with executing your tests in parallel

public static async Task Test()
{
    var successExecute = Execute("https://jsonplaceholder.typicode.com/posts", new { value = "ITWeiHan" }); //Result : {"value": "ITWeiHan","id": 101}
    var errorExecute = Execute("https://jsonplaceholder.typicode.com/error404", new { value = "ITWeiHan" }); //Result : Error 404
    await Task.WhenAll(successExecute, errorExecute);
}

Also, PostByJsonContentTypeAsync throws couple of more exceptions. Namely InvalidOperationException and ArgumentNullExceptions. Are you intentionally not handling them?


Fluent NHibernate, Getting or Creating a new `Tag` Edit

The rest of my code utilises the Command/Query seperation pattern. I'll probably move this out into a command and a query but for now I've written this directly in my MVC controller for brevity.

Original

I'm quite new to Fluent NHibernate and its various workings. This code is designed to receive an array of tag names, get those tags from the database and then create any tags that were in the array but not already in the database.

Here's the code I've just written:

private IEnumerable<Tag> GetOrCreateTags(IEnumerable<string> tagNames)
        {

            var tagCriteria = _nHibernateSession.CreateCriteria<Tag>();

            var or = Restrictions.Disjunction();

            foreach (var tagName in tagNames)
            {
                or.Add(Restrictions.Eq("Name", tagName));
            }
            tagCriteria.Add(or);

            var tagsFromTagNames = tagCriteria.List<Tag>();

            foreach (var newTagNeverSeen in tagNames.Except(tagsFromTagNames
                                                           .Select(x => x.Name)))
            {
                using (var transaction = _nHibernateSession.BeginTransaction())
                {
                    var newTag = new Tag
                                     {
                                         Name = newTagNeverSeen,
                                         Enabled = true,
                                         DateCreated = DateTime.Now
                                     };
                    _nHibernateSession.SaveOrUpdate(newTag);
                    tagsFromTagNames.Add(newTag);
                    transaction.Commit();
                }
            }
            return tagsFromTagNames;
        }

How would you improve this? I feel like I'm doing too much here already so I'll likely refactor this to split it out.

Admittedly I am using a really old version of nhibernate, but I think this is still there (to replace a few lines):

Restrictions.InG("Name", tagNames.ToList())

I think I would also bring the using statement outside of the loop:

private IEnumerable<Tag> GetOrCreateTags(IEnumerable<string> tagNames)
    {
        var tagList = tagNames.ToList(); //or make the parameter above an ICollection<string>

        var tagCriteria = _nHibernateSession.CreateCriteria<Tag>();
        tagCriteria.Add(Restrictions.InG("Name", tagList));

        var tagsFromTagNames = tagCriteria.List<Tag>();

        //naming this for readability
        var existingTagNames = tagsFromTagNames.Select(x => x.Name);

        var newTags = tagList
            .Except(existingTagNames)
            .Select(t => new Tag
                           {
                               Name = newTagNeverSeen,
                               Enabled = true,
                               DateCreated = DateTime.Now
                           }); //moving this outside the loop makes the loop cleaner

        if (newTags.Any())
        {
            using (var transaction = _nHibernateSession.BeginTransaction())
            {
                foreach (var tag in newTags)
                {
                    _nHibernateSession.SaveOrUpdate(tag);
                    tagsFromTagNames.Add(tag);
                }
                transaction.Commit();
            }
        }
        return tagsFromTagNames;
    }


Tic-Tac-Toe game requiring two players I have a working Tic-Tac-Toe game, which requires two players. The game also keeps track of the result of each game and assigns score to each player based on the result of the game.

For this, I have made use of structures:

Player Structure

typedef struct
{
    int score[10];
    char choice;
    char name[10];
    bool turn;

}Player;

Game Structure

I have made use of another structure for the game setup, which contains the game board, the valid positions available, and also the number of games played:

typedef struct
{
    char board[3][3];
    bool validPos[10];
    int games;

}Game;

main function

 // included only a part of the main function that I am concerned about;
int main(void)
{
    Game *game = malloc(sizeof(Game));
    Player *player_1 = malloc(sizeof(Player));
    Player *player_2 = malloc(sizeof(Player));
}

Functions used in the game:

void assignXO(Player *, Player *);
void startGame(Game *, Player *, Player *);
void display(Game *, Player *, Player *);
bool victoryCheck(Game *);
bool isComplete(Game *);
void playerMove(Game *, Player *);
void declareWinner(Game *, Player *, Player *);
void gameReset(Game *);
void scoreBoard(Player *, Player *, int);

Queries

As you can observe the number of arguments passed to the functions are dependent on the purpose of the function. Is it wise to declare global variables for the structure so that potentially no arguments are needed to be passed to any of the functions?
Is it good practice to have more than 2 arguments passed to the functions? Does it help with conveying the logic to the reader?
If I used the structure Player within the Game structure, will it make the code any better in terms of readability and how well the code is organised?

Well, it's a good idea to minimize dependency on global state.
Doing so enhances testability and reduces dependencies, especially the hidden type.

Still, if you have such pervasively used state as Game, it's a valid thought to make it global instead of passing it around everywhere.
Pass the arguments needed, no more, no less.
It could be. Or not. In any case, it would reduce your flexibility in using your code for example for a tic-tac-toe contest.

Now lets critic your code:

validPos seems to be redundant, being a simple function of board. Or is there some extra magic?
Avoid dynamic allocation where it doesn't help. That means the memory-used is scoped and the block is reasonably small, like those allocations in main.
Anyway, avoid repeating your types to avoid mismatches. It should have been:

Game *game = malloc(sizeof *game);

Parameter names are valuable space for documenting an arguments use.
At least scoreBoard and declareWinner could use a bit more:

void declareWinner(Game *, Player* winner, Player* looser);
void scoreBoard(Player *, Player *, int round);
// documented my guesses what the pertinent arguments mean


Timing word-searching code in Python I'm using a linear search as a spell checker. I need to time my code to get averages as I'm using different searches and comparing them. However I don't believe the code I've used to time the code is accurate enough or the best method. If anyone has any suggestion that would be great.

import re
import time

start_time = time.time()
f1=open('writefile.txt', 'w+')

def linS(target, words):
    '''linear search for target in words. words need not be sorted'''
    for s in words:
        if s==target:
            return True
    return False

words = [s.strip("\n").lower() for s in open("dict.txt")]
words.sort() # sort the list

with open("text.txt" , encoding="utf8") as text:
    content = text.read().split(" ")
    content = [item.lower() for item in content]
    content = ' '.join(content)
    content = re.findall("[a-z]+", content)

for w in content:
    if not linS(w, words):
        f1.write(w+'\n')

f1.close()

print("--- %s seconds ---" % (time.time() - start_time))

Some suggestions:

Pass the code through pycodestyle to make it more idiomatic.
Use with open() (or try/finally for a more complicated solution) to make sure files are closed whenever they are done with.
You can do .readlines() on a file pointer to avoid stripping newlines.
The dictionary is presumably controlled by you. Sort and lower-case it once, and then you don't have to do that every time the script runs.
These are logically equivalent:

    for x in y:
        if x == target:
            return True
    return False

and

    return target in y

Use descriptive names such as for word in words and `def linear_search.
Put the timing code around the lines you are trying to time. If you want to know the aggregate time of linS then get the start and end times just before and just after the last loop (which will count the time it takes to write to disk buffers) or accumulate a sum of run times within `linS`.


LeetCode: Trie Tree implementation, Search, Insert, startWith C# https://leetcode.com/problems/implement-trie-prefix-tree/

Please comment about performance and style.

  Implement a trie with insert, search, and startsWith methods.

  Example:

Trie trie = new Trie();

trie.insert("apple");
trie.search("apple");   // returns true
trie.search("app");     // returns false
trie.startsWith("app"); // returns true
trie.insert("app");
trie.search("app");     // returns true

  Note:

  You may assume that all inputs are consist of lowercase letters a-z.
  All inputs are guaranteed to be non-empty strings.

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

namespace TrieQuestions
{
    /// <summary>
    /// https://leetcode.com/problems/implement-trie-prefix-tree/
    /// </summary>
    [TestClass]
    public class TrieTreeImplementation
    {
        [TestMethod]
        public void TrieInsertTest()
        {
            Trie trie = new Trie();
            trie.Insert("cat");
            Assert.IsTrue(trie.Search("cat"));
        }

        [TestMethod]
        public void TriePrefixSearchTest()
        {
            Trie trie = new Trie();
            trie.Insert("cats");
            Assert.IsTrue(trie.StartsWith("cat"));
        }

        [TestMethod]
        public void OneLetterEdgeCaseTest()
        {
            Trie trie = new Trie();
            trie.Insert("a");
            Assert.IsTrue(trie.Search("a"));
            Assert.IsTrue(trie.StartsWith("a"));
        }
    }

    public class Trie
    {
        public TrieNode Head { get; set; }

        /** Initialize your data structure here. */
        public Trie()
        {
            Head = new TrieNode();
        }

        /** Inserts a word into the trie. */
        public void Insert(string word)
        {
            var current = Head;
            for (int i = 0; i < word.Length; i++)
            {
                if (!current.Edges.ContainsKey(word[i]))
                {
                    current.Edges.Add(word[i], new TrieNode());
                }
                current = current.Edges[word[i]];
            }
            current.IsTerminal = true;
        }

        /** Returns if the word is in the trie. */
        public bool Search(string word)
        {
            var current = Head;
            for (int i = 0; i < word.Length; i++)
            {
                if (!current.Edges.ContainsKey(word[i]))
                {
                    return false;
                }
                current = current.Edges[word[i]];
            }
            return current.IsTerminal == true;
        }

        /** Returns if there is any word in the trie that starts with the given prefix. */
        public bool StartsWith(string prefix)
        {
            var current = Head;
            for (int i = 0; i < prefix.Length; i++)
            {
                if (!current.Edges.ContainsKey(prefix[i]))
                {
                    return false;
                }
                current = current.Edges[prefix[i]];
            }
            return true;
        }
    }

    public class TrieNode
    {
        public Dictionary<char, TrieNode> Edges { get; set; }
        public bool IsTerminal { get; set; }
        public TrieNode()
        {
            Edges = new Dictionary<char, TrieNode>();
        }
    }
}

In terms of data structures and algorithms this all looks pretty straightforward - not much to say about that.

Performance

Edges.ContainsKey and Edges[...] each perform a lookup. Edges.TryGetValue lets you achieve the same with just a single lookup.

Design

I see no reason why Trie.Head should be public, and certainly not why it should have a public setter. That's poor encapsulation. Likewise, TrieNode.Edges should be get-only: you don't want outside code to be able to do Edges = null;.
Search and StartsWith do exactly the same thing, except for the final check. I'd move the duplicate code to a TrieNode FindNode(string prefix) helper method.
TrieNode is only used internally within Trie, so it makes sense to make it a private inner class.

Other notes

You can remove Trie's constructor if you initialize Head directly: TrieNode Head { get; } = new TrieNode();. The same goes for TrieNode and Edges.
I'd replace those for loops with foreach loops, for clarity's sake.
Comparing a boolean against true is unnecessary. Just do return current.IsTerminal;
I'd replace those default LeetCode comments with C#-specific xml comments.


ImGui manager singleton For my game engine I am implementing ImGui for debugging purposes. In order to manage ImGui I have made a singleton to make sure only one ImGui context gets created. I am not entirely sure if this is a good implementation of a Singleton so I am looking for some feedback, also is using a Singleton here the right way or could I better use a full static class?

ImGUIManager.h

#ifndef CHEETAH_CORE_IMGUIMANAGER_H_
#define CHEETAH_CORE_IMGUIMANAGER_H_

#include "Core.h"
#include "../ImGUI/imgui.h"

#include <memory>

namespace cheetah
{
    class CH_API ImGUIManager
    {
    public:
        static void init();
        static void shutDown();
        static void begin();
        static void end();

        static inline ImFont* getTextFont() { return ImGUIManager::getInstance()->m_text; };
        static inline ImFont* getIconSmallFont() { return ImGUIManager::getInstance()->m_iconSmall; };
        static inline ImFont* getIconLargeFont() { return ImGUIManager::getInstance()->m_iconLarge; };

        virtual ~ImGUIManager() = default;

    protected:
        ImGUIManager() = default;
        virtual void init_impl() = 0;
        virtual void shutDown_impl() = 0;
        virtual void begin_impl() = 0;
        virtual void end_impl() = 0;

    protected:
        ImFont* m_text = nullptr;
        ImFont* m_iconSmall = nullptr;
        ImFont* m_iconLarge = nullptr;

    private:
        static ImGUIManager* getInstance();
        static void create();

    private:
        static std::unique_ptr<ImGUIManager> m_instance;
    };
}

#endif // !CHEETAH_ENGINE_IMGUI_IMGUIMANAGER_H_

ImGUIManager.cpp

#include "ImGUiManager.h"
#include "Renderer/RenderAPI.h"
#include "Platform/OpenGL/OpenGLImGUIManager.h"

namespace cheetah
{
    std::unique_ptr<ImGUIManager> ImGUIManager::m_instance = nullptr;

    void ImGUIManager::init()
    {
#ifdef DEBUG
        static bool initialized = false;
        ASSERT(!initialized, "ImGUI is already initialized");
#endif // DEBUG
        ImGUIManager::getInstance()->init_impl();
#ifdef DEBUG
        initialized = true;
#endif
    };

    void ImGUIManager::shutDown()
    {
        ImGUIManager::getInstance()->shutDown_impl();
    };

    void ImGUIManager::begin()
    {
        ImGUIManager::getInstance()->begin_impl();
    };

    void ImGUIManager::end()
    {
        ImGUIManager::getInstance()->end_impl();
    };

    ImGUIManager* ImGUIManager::getInstance()
    {
        if (!m_instance)
            ImGUIManager::create();

        return m_instance.get();
    }

    void ImGUIManager::create()
    {
        switch (RenderAPI::getAPI())
        {
        case RenderAPI::API::OpenGL:
            m_instance = std::make_unique<opengl::OpenGLImGUIManager>();
            break;
        default:
            m_instance = nullptr;
        }
    }
}

Its not thread safe!

Looks like you can still create instances manually (though you would have to derive a class that can use the protected constructor) or copy the one created by getInstance().

Why not use the classic Singleton pattern?

class ImGUIManager
{
     public:
         static ImGUIManager& getInstance()
         {
             static ImGUIManager instance;   // static member
                                             // Correctly constructed on first use.
                                             // Will be deleted on exit.
             return instance;
         }
         // PUT YOUR INTERFACE HERE

     private:
         ImGUIManager(){} // Make sure the constructor is private.
         // Delete the copy and move constructors.
         ImGUIManager(ImGUIManager const&)            = delete;
         ImGUIManager& operator=(ImGUIManager const&) = delete;
         ImGUIManager(ImGUIManager&&)                 = delete;
         ImGUIManager& operator=(ImGUIManager&&)      = delete;
};

Some References:

C++ Singleton design pattern


Is there a better way to write these two nested loops in Erlang? Since one month, I'm learning Erlang and I like it so much.
Today, I wrote an algorithm to solve a problem.
Some time ago, I used two nested for loops in C# to solve it but, solving the same problem in Erlang, I asked myself if there is a better way to write something similar to the two nested for loops I used before.

Let's say that the two nested for loops in C# are:

for(int i = 100; i<=999;i++)
    {
        for(int j = 100;j<=999;j++)
        {
            int prod = i*j;
              //some logic here..
        }
    }

and the respective code in Erlang is the following:

loopINumber(_,LowerLim,UpperLim,Product) when LowerLim > UpperLim -> {Product};

loopINumber(ILimit,LowerLim,UpperLim,Product) ->
    Product = loopJNumber(ILimit,UpperLim,LowerLim,Product),
    loopINumber(ILimit,LowerLim+1,UpperLim,Product).

loopJNumber(LowerLim,UpperLim,_,Product) when LowerLim > UpperLim -> Product;

loopJNumber(LowerLim,UpperLim,ILimit,Product) ->
    Product = ILimit * LowerLim,
    %% some logic here..
    loopJNumber(LowerLim+1,UpperLim,ILimit,Product).

start(ILimit,JLimit) -> loopINumber(ILimit,ILimit,JLimit,0).

I know that maybe there's something to improve. Being still a beginner in functional languages, most probably I'm still coding in a "sequential" or OOP way. How can I improve this Erlang code?

In general, nested for loops translate well to list comprehensions:

Prods = [I*J || I <- lists:seq(100,999), J <- lists:seq(100,999)]
// Generates a list of products to perform further logic on


Identifying originating country of an IP address I have a small assignment to read IP addresses and from a bunch of external text files identify which country the IP address comes from. I opted to use XmlReader and read the ranges from an XML file but unfortunately the performance has been quite abysmal (an average search time of around 7000 milliseconds).

Obviously serializing the ranges isn't an option as we're talking about 142,222 entries. Is there anything I can do to optimize my method or is there a quicker way to search this, without resorting to SQL?

class IPRangeRepository : IDisposable
    {
        public IPRangeRepository(string fileName)
        {
            this.fileName = fileName;
            xmlReader = XmlReader.Create(fileName);
        }

        private XmlReader xmlReader;
        private string fileName;

        public string GetIPCountry(IPAddress adress)
        {
            while (xmlReader.Read())
            {
                // Ok instantiate object and then do comparison...
                if (xmlReader.Name == "IPRange") {
                var ipRange = IPRange.TryParse(xmlReader["Value"]);
                if (ipRange.IsIPAddressInRange(adress)) return xmlReader["Country"];
                }
            }

            //Return empty string if we don't find anything
            return string.Empty;
        }

        public void Dispose()
        {
            xmlReader.Dispose();
        }
    }

the only issues that I see with this code so far is formatting.

class IPRangeRepository : IDisposable
    {
        public IPRangeRepository(string fileName)
        {
            this.fileName = fileName;
            xmlReader = XmlReader.Create(fileName);
        }

This indentation is not standard, it should look like this

class IPRangeRepository : IDisposable
{
    public IPRangeRepository(string fileName)
    {
        this.fileName = fileName;
        xmlReader = XmlReader.Create(fileName);
    }

and your if statements inside the while loop are not easy to read.

        while (xmlReader.Read())
        {
            // Ok instantiate object and then do comparison...
            if (xmlReader.Name == "IPRange") {
            var ipRange = IPRange.TryParse(xmlReader["Value"]);
            if (ipRange.IsIPAddressInRange(adress)) return xmlReader["Country"];
            }
        }

this is how I prefer to write this code

while (xmlReader.Read())
{
    // Ok instantiate object and then do comparison...
    if (xmlReader.Name == "IPRange") {
        var ipRange = IPRange.TryParse(xmlReader["Value"]);
        if (ipRange.IsIPAddressInRange(adress))
        {
            return xmlReader["Country"];
        }
    }
}

the return is a little too complex for something that I would one line, and most of the time if I can one line something it's going to look better as a ternary statement, this isn't one of those instances.

Always indent code inside of any block.


Find log4net logfiles at runtime without adding dependency to project I was searching for a way to get all active log4net logfiles (if any) at runtime without adding a reference to the project.

This is the solution I came up with:

private static IEnumerable<AbstractReportInformation> GetLog4NetLogfiles()
{
    var type = Type.GetType("log4net.LogManager,log4net");
    if(null == type)
        return new List<AbstractReportInformation>();

    dynamic[] repositories = type.GetMethod("GetAllRepositories").Invoke(null, null) as dynamic[];

    IEnumerable<FileInfo> files = repositories
        .SelectMany<dynamic, dynamic>(repos => repos.GetAppenders())
        .Where(appender => Type.GetType("log4net.Appender.FileAppender,log4net").IsAssignableFrom(appender.GetType()))
        .Select(appender => new FileInfo(appender.File));

    return files.Select(
        file => new FilesystemInformation(file)).Cast<AbstractReportInformation>().ToList();
}

It makes use of the dynamic keyword instead of doing it all with reflection.

Are there any obvious errors in my code?
Are there better ways to
achieve the same result?

I feel a bit unsure about leaving the safety of the otherwise strongly typed language / framework.

Update to explain why no references shall be used / why dynamic is used

The above code is used to "detect" whether log4net is present and then gathers the logfiles. The same will later be implemented for NLog and possibly other frameworks of that sort. The project I'm using this code in is a application health monitoring framework and I don't want to add a lot of references.

Well, that seems a strange solution to me. Despite the fact you don't have a formal reference in References folder of a VS project you still have a dependency on the log4net library. And if in runtime the log4net library won't be available, your code will fail. Moreover, somebody has to remember about that dependency and copy log4net.dll manually to you bin folder, so the code could be executed.

From my point of view, it is always better to have a visible dependency rather than having invisible one. The only exception comming to my mind is when you need to have a modular architecture where modules can be safely plugged and unplugged. And, as far as I can understand, it is not your requirement.

I would recommend you to create a separate project that would have a normal reference to log4net and which would be used by your main project. In that case you will have your main project unaware of log4net, but in the same time you would have all compile-time checks and every benefit of strongly typed language / framework.


Implementation of Point, Velocity and Acceleration in Haskell I'm trying to implement point, velocity and acceleration types.

They should be connected by some derive function which:

takes time and velocity and returns a point increment;
takes time and acceleration and returns a velocity increment.

In pseudocode, it should look like this:

derive :: Time -> Velocity -> Point
derive :: Time -> Acceleration -> Velocity

Time is a type representing a time as floating value.

Point, Velocity and Acceleration are vectors.

So I don't want to

mix time values with any other floating values;
mix vectors representing points with velocity vectors and etc.

I came up with following solution.

{-# LANGUAGE KindSignatures, DataKinds, TypeOperators #-}

import Control.Comonad

import GHC.TypeLits
import Linear
import Linear.V2

-- Type of real numbers.
type R = Double

-- Type of vectors.
type Vector = V2

-- Wrapper to distinguish time values from other values.
newtype Time a = Time { fromTime :: a }

-- Time is intended to be a wrapper. But to implement a derive function,
-- I need a common way to extract value from wrapper. That's why Time
-- must be a Comonad's instance:

instance Functor Time where
    fmap f = Time . f . fromTime

instance Comonad Time where
    extract = fromTime
    duplicate = Time

{- Type of derivative.

     Type (D r v u a) means a derivation of (u a) by (v a) with rank r.

-}
newtype D (r :: Nat) (v :: * -> *) (u :: * -> *) a = D { fromD :: (u a) }

-- Using type D the point, velocity and acceleration types can be defined:
type Pnt = D 0 Time Vector R
type Vel = D 1 Time Vector R
type Acc = D 2 Time Vector R

-- Even if I don't want to mix points with velocities,
-- I do want them to behave like vectors. So I want
-- them to be Additive:

instance Functor u => Functor (D r v u) where
    fmap f = D . fmap f . fromD

instance Additive u => Additive (D r v u) where

    -- I didn't found a way how to make this Additive instance better.
    -- Applicative instance for (D r v u) doesn't help.

    zero = D $ zero

    x ^+^ y = D $ (fromD x) ^+^ (fromD y)
    x ^-^ y = D $ (fromD x) ^-^ (fromD y)

    lerp a x y = D $ lerp a (fromD x) (fromD y)

    liftU2 f x y = D $ liftU2 f (fromD x) (fromD y)
    liftI2 f x y = D $ liftI2 f (fromD x) (fromD y)

-- Now derive function can be implemented:
derive ::
    (Comonad v, Functor u, Num a) =>
        v a -> D (r + 1) v u a -> D r v u a

derive dv du = D $ (extract dv) *^ (fromD du)

This solution pretty mush does what I want:

I can't call derive on Pnt;
derive on Vel returns Pnt;
derive on Acc returns Vel.

I don't like:

the way how Additive instance for D r v u is implemented.
the fact that to derive a vector of real numbes I need to extract time value from a wrapper. It doesn't feel natural.

So ...

How can I edit an Additive instance for D r v u to avoid using common code with fromD?
Am I wrong about naturality which I meantioned above; is it ok to use wrappers like Time like I did in derive function?

Any suggestions are appreciated.

Use GeneralizedNewtypeDeriving if you use the same instance as the wrapped type anyway. Together with DeriveFunctor, we can get rid of much boilerplate (comments omitted, changes noted with -- <--):

{-# LANGUAGE KindSignatures, DataKinds, TypeOperators #-}
{-# LANGUAGE GeneralizedNewtypeDeriving, DeriveFunctor #-}                -- <--

import Control.Comonad

import GHC.TypeLits
import Linear
import Linear.V2

type R = Double
type Vector = V2

newtype Time a = Time { fromTime :: a } deriving (Functor, Num)           -- <--

instance Comonad Time where
    extract = fromTime
    duplicate = Time

newtype D (r :: Nat) (v :: * -> *) (u :: * -> *) a = D { fromD :: (u a) }
    deriving (Functor, Applicative, Additive)                             -- <--

type Pnt = D 0 Time Vector R
type Vel = D 1 Time Vector R
type Acc = D 2 Time Vector R

derive ::
    (Comonad v, Functor u, Num a) =>
        v a -> D (r + 1) v u a -> D r v u a

derive dv du = D $ (extract dv) *^ (fromD du)

This immediately gets rid of your Additive problem. Also, now that Time is a Num instance, you can use derive like this:

derive 3 (pure 3 :: Vel)

Unfortunately, Num also contains (*), so you'll be able to multiply time values, which doesn't really make sense in this case. Alas, there is no other way to get fromInteger otherwise.

However, let's have a look at the wrapper, D and derive. The type of derive is a little bit too general. It allows you to do stuff like this:

ghci> import Data.Tree
ghci> derive (Node 10 []) (pure 2 :: D 2 Tree Vector Double)
D {fromD = V2 20.0 20.0} -- if we add Show to D's instances

Could this be a valid use case? Who knows. Is it likely to be a valid use case? Rather not. Get back to the original inspiration for derive:

derive :: Time -> Velocity     -> Point
derive :: Time -> Acceleration -> Velocity

We expect the first argument always to be some kind of time measurement. We also expect the vector to be conform to the time's unit(*). From this point of view, it makes sense to encode the Time in D. However, this yields the question why the other side of the spacetime, space, isn't encoded in D too.

(*) strictly speaking, we're not encoding time's unit but only type.

Either way, you probably want to allow only Time values. Furthermore, you want to make clear what unit Time uses. We could use a phantom type, e.g.

{-# LANGUAGE DataKinds, KindSignatures, GeneralizedNumtypeDeriving #-}
data TimeUnit = Milliseconds | Seconds | Minutes | Hours

newtype Time (* :: TimeUnit) a = Time {fromTime :: a} deriving (Show, Num)

which makes sure that we don't mix different times:

> (Time 3 :: Time 'Seconds Double) + (Time 10 :: Time 'Minutes Double)
Couldn't match type ‘'Minutes’ with ‘'Seconds’
Expected type: Time 'Seconds Double
  Actual type: Time 'Minutes Double
In the second argument of ‘(+)’, namely
  ‘(Time 10 :: Time Minutes Double)’
In the expression:
  (Time 3 :: Time Seconds Double) + (Time 10 :: Time Minutes Double)

But that's out of scope for this review. Instead, we can simply say that Time's unit is seconds, disallow creating Time values with its data constructor, and provide some helper functions:

module Movement
 ( Time, getSeconds
 , seconds, minutes
 , ...
 )
where

newtype Time a = Time { getSeconds :: a }

seconds :: Num a => a -> Time a
seconds = Time

minutes :: Num a => a -> Time a
minutes = seconds . (60 *)

derive :: (Functor u, Num a) => Time a -> D (r + 1) u a -> D r u a
derive (Time s) du = D $ s *^ (fromD du)

Alright. Let's reflect all changes:

used GeneralizedNewtypeDeriving to get rid of boilerplate (major change)
instead of allowing general Comonads, only allow Time (*)
removed Time from D, since derive fixes it  (*)
make the unit of time explicit with "smart" constructors and remove the Time data constructor from the exports.

(*) You can still use those techniques internally, but a public interface should be hard to use wrong if possible. After all, that's the reason you're using r :: Nat and derive :: ... -> D (r + 1) ... -> D r, right?

We end up with the following code:

{-# LANGUAGE KindSignatures, DataKinds, TypeOperators #-}
{-# LANGUAGE GeneralizedNewtypeDeriving, DeriveFunctor #-}
module Movement
 ( Time, getSeconds
 , R, Vector,
 , D(..), Pnt, Vel, Acc
 , seconds, minutes
 , derive
 )
where

import GHC.TypeLits
import Linear
import Linear.V2

type R = Double
type Vector = V2

newtype Time a = Time { getSeconds :: a } deriving (Functor)

newtype D (r :: Nat) (u :: * -> *) a = D { fromD :: (u a) }
    deriving (Functor, Applicative, Additive, Show)

type Pnt = D 0 Vector R
type Vel = D 1 Vector R
type Acc = D 2 Vector R

seconds :: Num a => a -> Time a
seconds = Time

minutes :: Num a => a -> Time a
minutes = seconds . (60 *)

derive :: (Functor u, Num a) => Time a -> D (r + 1) u a -> D r u a
derive (Time s) du = D $ s *^ (fromD du)

  Am I wrong about naturality which I meantioned above; is it ok to use wrappers like Time like I did in derive function?

Using wrappers is fine. For example, the UniversalTime in the time package is just a wrapper around Rational. And although it doesn't export its constructor, DiffTime is also just a newtype of Pico.

Maybe it feels more natural with the helper functions:

someCalculation :: Vec
someCalculation =
  let time = seconds 120
      acc  = D (V2 10 10) -- (*)
  in derive time acc

(*) This example will probably inspire you to provide Num a => a -> a -> D r V2 a functions.


Compare function for lists I need to sort a list of lists. The following is a compare function:

def compare(lhs, rhs): # lhs&rhs are lists
    delta = len(lhs) - len(rhs)
    if delta > 0:
        return 1
    elif delta < 0:
        return -1
    if lhs < rhs:
        return 1
    elif lhs > rhs:
        return -1
    return 0

It looks pretty wordy. Is there a way to rewrite the function without performance penalty?

If I understand correctly, you want to sort lists by their length first, and then by "whatever lists are otherwise sorted by". I also assume that you want to pass your function as the cmp argument to the list.sort or sorted function.

You can do this in both a more natural to read, and computationally more efficient way, by using the key argument instead:

  In general, the key and reverse conversion processes are much faster
  than specifying an equivalent cmp function. This is because cmp is
  called multiple times for each list element while key and reverse
  touch each element only once.

(Source: https://docs.python.org/2/library/stdtypes.html#mutable-sequence-types)

This should be the appropriate key function for your case:

def length_first(lst):
    return len(lst), lst

Or simply:

lambda lst: len(lst), lst

Example:

>>> from random import randrange
>>> lists = [[randrange(5) for _ in range(randrange(3, 6))]
...          for _ in range(10)]
>>> print '\n'.join(map(str, sorted(lists, key=lambda lst: (len(lst), lst))))
[0, 0, 4]
[2, 3, 0]
[2, 4, 0]
[3, 4, 2]
[4, 1, 4]
[1, 0, 1, 0]
[2, 0, 1, 1]
[3, 1, 0, 4]
[0, 0, 0, 3, 2]
[4, 0, 1, 2, 1]


Optimizing a vector loop Visual Studio somehow optimizes the below code to be 20 times faster (release with optimization vs. release with no optimization). What could it be doing?

for (unsigned n = 1; n != units.size(); n++)
        for (unsigned j = 0; j != (units.size() - n); j++)
        {
            unsigned sizesMap[11] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };

            for (auto it = units.begin() + n; it != (units.begin() + (n + j + 1)); it++)
            {
                for (unsigned k = 1; k != 11; k++)
                {
                    sizesMap[k] += (*it).sizes[k];

                }
            }
            //do something with sizesmap
        }

Unit class has a sizes array (10 members). The first two loops give all the possible sequences of "unit" in the vector. (1-2, 1-3, 1-100, 2-3, 2-4, 2-100, etc), which I then compare with one another using the sizesmap. I tried unrolling the inner loop and just adding sizesmap 10 times, that gave about 2% performance improvement. I compiled with Visual Studio 2013. Could it be vectorization (adds the (*it).sizes to sizesmap in one go)? Or are the loops ineffective somehow?

Educated guesses are: in this case, loop unrolling will probably feature in a big way, and may account for most of the 20X improvement. Additionally, with the loop unroll it may make other things like instruction-level-parallelism more effective, and even cache-line management.
There are essentially only two comments to make on your situation:

non-optimized code is going to be slower.... you pay money for the compiler for a reason, lots of smart people make it 'go fast'.
you need to inspect the instruction-level code (asm) generated by the compiler in order to understand the main differences between the two versions ... anything else is just educated guessing.

Edit
FWIW, Purely for interest sake.... , you may want to try to manually unroll your inner-most loop, and see if this makes a significant difference:
        for (auto it = units.begin() + n; it != (units.begin() + (n + j + 1)); it++)
        {
            sizesMap[1] += (*it).sizes[1];
            sizesMap[2] += (*it).sizes[2];
            sizesMap[3] += (*it).sizes[3];
            sizesMap[4] += (*it).sizes[4];
            sizesMap[5] += (*it).sizes[5];
            sizesMap[6] += (*it).sizes[6];
            sizesMap[7] += (*it).sizes[7];
            sizesMap[8] += (*it).sizes[8];
            sizesMap[9] += (*it).sizes[9];
            sizesMap[10] += (*it).sizes[10];
            sizesMap[11] += (*it).sizes[11];
        }


Verifying if one list contains a string from another list I am trying to validate that when given two lists,

List<String> models;
List<String> titleOfVehicle;

that one of the models must be contained in each of the titleOfVehicle.

For example, given the following:

models = {Accord, Civic, Element}
titleOfVehicle1 = {2014 Honda Accord, 2011 Toyota Camry}
titleOfVehicle2 = {2013 Honda Accord, 2015 Honda Element, 2011 Honda Civic}

then when comparing models and titleOfVehicle1 it will fail since 2011 Toyota Camry does not contain one of the models provided. However, titleOfVehicle2 will pass since it contains a model for each vehicle titles.

Here is my code:

public boolean compareTwoList(List<String> models, List<String> titleOfVehicles) {
    boolean flag = false;

    for(int i = 0; i < titleOfVehicles.size(); i++) {
        flag = false;
        int k = 0;
        for(int j = 0; j < models.size(); j++) {
            if(titleOfVehicles.get(i).contains(models.get(j))) {
                flag = true;
                break;
            } else if(k == models.size() - 1 && flag == false) {
                return false;
            }
            k++;
        }
    }
    return flag;
}

The current code works but it is very awkward, as rolfl pointed out. I would add that you should never compare booleans with true or false, like what you're doing in:

} else if(k == models.size() - 1 && flag == false) {
    return false;
}

Instead, have

} else if (k == models.size() - 1 && !flag) {
    return false;
}

You can actually accomplish this task in a single, clear and easy line using the Stream API:

public boolean compareTwoList(List<String> models, List<String> titleOfVehicles) {
    return titleOfVehicles.stream().allMatch(t -> models.stream().anyMatch(t::contains));
}

This does exactly what is written: it returns whether all elements in the given titles contains any of the given models.

Both allMatch and anyMatch are short-circuiting operations, so it will behave exactly like your current code.


Counting sort in Python I wrote the following function to implement counting sort:

def CountSort( Arr, Max ):
    counter, retVal = [0] * (Max + 1), []
    for _ in Arr:
        counter[_] += 1
    for i in xrange( Max + 1 ):
        retVal += [i] * counter[i]
    return retVal

NOTE

The function takes the array of integer/list of integer as its first argument, and the maximum value it stores as the second. The only restriction is that the data in array should be distributed for \$0 \le x \le Max\$.

I'm not happy with it since I have to maintain two lists in memory now, namely Arr and retVal. I could, obviously, use the following:

    # indentation preserved
    pos = 0
    for i in xrange( Max + 1 ):
        while counter[i] > 0:
            Arr[pos] = i
            pos += 1
            counter[i] -= 1

but it won't be as pythonic as the first one. An alternate approach is to delete the input Arr, and recreate it:

    # indentation preserved
    del Arr
    Arr = []
    for i in xrange( Max + 1 ):
        Arr += [i] * counter[i]

but this is not dissimilar to the second approach.

I'm open to suggestions on improvement of the first (or any of the latter).

Well, you kind of have to have two arrays. You need your counts array and you need your output array. If you want to sort the input array in place, you can do that, that's just a completely different semantic for your algorithm. So you'll just need to figure that out up front: do you want to sort the input list (a la list.sort()) or do you want to return a new list that is sorted (a la sorted()).

That said, here's some comments on your initial solution. To start with:

counter, retVal = [0] * (Max + 1), []

There is no reason to do such a thing. That is unnecessary terse and you're just making it hard to see what's going on. Furthermore, you don't even need retVal for now. So let's just start with the counter by itself:

counter = [0] * (Max + 1)
for _ in Arr:
    counter[_] += 1

Next, we have this loop:

for i in xrange( Max + 1 ):
    retVal += [i] * counter[i]

You can actually do a little better in terms of code appearance. We have our counter array, which has all of those indices... what we want to do is iterate over the counter array while also having access to the indices. That's what enumerate() is for:

retVal = []
for i, count in enumerate(counter):
    retVal += [i] * count
return retVal

Lastly, if you want to follow PEP-8, your function name should really be counting_sort(). And _ should probably be replaced with some single-letter name (x, a, elem, what have you).


Temperature conversion application I'm two months in a programming course and our instructor told us we should think about building real-world applications as to gain valuable experience. I got the idea of doing a simple temperature conversion app.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace TempratureProject
{
    class Program
    {
        static void Main(string[] args)
        {
            //Ask for the temprature that the user wants to convert
            Console.WriteLine("What is the temprature you wish to convert?");
            var RInput = Console.ReadLine();
            double dInput = Convert.ToDouble(RInput);
            //Ask for what type of conversion
            Console.WriteLine("If you wish to convert Fahrenheit to Celsius, Press 1");
            Console.WriteLine("If you wish to convert Fahrenheit to Kelvin, Press 2");
            Console.WriteLine("If you wish to convert Celsius to Fahrenheit, Press 3");
            Console.WriteLine("If you wish to convert Celsius to Kelvin, Press 4");
            Console.WriteLine("If you wish to convert Kelvin to Fahrenheit, Press 5");
            Console.WriteLine("If you wish to convert Kelvin to Celsius, Press 6");
            //Set YInput equal to the choice made
            var YInput = Console.ReadLine();
              int iInput = Convert.ToInt32(YInput);
            //Use of a a series of If statements to call the methods
              if (iInput == 1)
              {
                  var aAnswer = FtoCelsius(dInput);

                  Console.WriteLine("The value of {0} degrees Fahrenheit is equall to {1} degrees Celsius", dInput, aAnswer);
              }
              else if (iInput == 2)
              {
                  var bAnswer = FtoKelvin(dInput);

                  Console.WriteLine("The value of {0} degrees Fahrenheit is equall to {1} degrees Kelvin", dInput, bAnswer);
              }
              else if (iInput == 3)
              {
                  var cAnswer = CtoFahenreit(dInput);

                  Console.WriteLine("The value of {0} degrees celcius is equall to {1} degrees Fahrenheit", dInput, cAnswer);
              }
              else if (iInput == 4)
              {
                  var dAnswer = CtoKelvin(dInput);

                  Console.WriteLine("The value of {0} degrees celsius is equall to {1} degrees Kelvin", dInput, dAnswer);
              }
              else if (iInput == 5)
              {
                  var eAnswer = KtoFahrenheit(dInput);

                  Console.WriteLine("The value of {0} degrees Kelvin is equall to {1} degrees Fahrenheit", dInput, eAnswer);
              }
            else
              {
                  var fAnswer = KtoCelsius(dInput);

                  Console.WriteLine("The Value of {0} degrees Kelvin is equall to {1} degrees Celsius", dInput, fAnswer);
              }

        }
        //The various methods
        static double FtoKelvin(double CTempIn)
        {
            double fKel = (CTempIn - 32) * (5 / 9) + 273.15;

            return fKel;
        }
        static double FtoCelsius(double CTempIn)
        {
            Double fCels = (CTempIn - 32) * (5 / 9);

            return fCels;
        }
         static double CtoFahenreit(double CtempIn)
        {
            double dFahr = (1.8) * (CtempIn + 32);

            return dFahr;
        }
         static double CtoKelvin(double CTempIn)
        {
            double cKel = CTempIn + 273.15;

            return cKel;
        }
         static double KtoFahrenheit(double CTempIn)
         {
             double kFah = (CTempIn - 273.15) * (9 / 5) + 32;

             return kFah;
         }
         static double KtoCelsius(double CTempIn)
         {
             double KCel = CTempIn - 273.15;

             return KCel;
         }
    }
}

It is working so there is that but I would like to know how well this is written and how I could make it better.

UI experience

Your menue text is hard to read and not intuitively clear. I'd prefer to see something like that:

     Console.WriteLine(
@"Choose to convert temperature values, enter a key please:\n
1 - Fahrenheit to Celsius
2 - Fahrenheit to Kelvin
3 - Celsius to Fahrenheit
4 - Celsius to Kelvin
5 - Kelvin to Fahrenheit
6 - Kelvin to Celsius, Press 6
> ");

Also outputting the text that way would make the code more readable.

Taking input from console

Your statements

 var YInput = Console.ReadLine();
 int iInput = Convert.ToInt32(YInput);

can be simplified to

 int iInput = Convert.ToInt32(Console.ReadLine());

consider to catch exceptions or use a String and TryParse() to check for invalid input.1

Rather use switch than if else cascades

You should replace

 if (iInput == 1)
          {
              var aAnswer = FtoCelsius(dInput);

              Console.WriteLine("The value of {0} degrees Fahrenheit is equall to {1} degrees Celsius", dInput, aAnswer);
          }
 else if (iInput == 2)
          {
              var bAnswer = FtoKelvin(dInput);

              Console.WriteLine("The value of {0} degrees Fahrenheit is equall to {1} degrees Kelvin", dInput, bAnswer);
          }

with

switch(iInput) {
case 1:
    var aAnswer = FtoCelsius(dInput);
    Console.WriteLine("The value of {0} degrees Fahrenheit is equall to {1} degrees Celsius", dInput, aAnswer);
    break;
case 2:
    var bAnswer = FtoKelvin(dInput);
    Console.WriteLine("The value of {0} degrees Fahrenheit is equall to {1} degrees Kelvin", dInput, bAnswer);
    break;
}

That makes your code more readable and concise.

Write generic code, if it's prone to be extended

At least I'd try to make all of that more generic, using a

public delegate double ConversionDelegate(double x);
Dictionary<Tuple<string,string>,ConversionDelegate) = {
    { {"Fahrenheit","Celsius"}, FtoCelsius } ,
    { {"Fahrenheit","Kelvin"}, FtoKelvin} ,
    // ...
    { {"Kelvin", "Celsius" }, KtoCelsius ,
};

dictionary pair. Where the tuple key serves the possible combinations of Fahrenheit to Celsius, Kelvin to Fahrenheit, etc. You can use the functions from Dictionary to match the key, and call the delegate and render the output accordingly.

The command line input should look like this then

> Enter temperature and <unit> <conversion unit>
> 39.5 celsius kelvin
> The value of 39.5 degrees celsius is equal to 312,65 degrees Kelvin

1)BTW you rather want to deal with double values IMO.


Assign monthly interest rate based on credit card input I have currently been working on a program for a few days now, and I am just about done.  The goal of my program is to ask the user for the amount they still owe on their credit card, and assign an appropriate monthly interest rate.  I have commented the code to make it a bit easier to understand, and I believe it is correct for the most part.  My only issue is I do not believed I have called my two other methods properly in Main.

import java.util.*;

/**
* Creates an account statement for a client of a credit card company based
* on their input.
*/

public class CSCard {

private static double interestRate;
private static double newBalance;
private static double interest;
public static void main(String[] args) {
    Scanner keyboard;
    double priorBalance;
    double addtlCharges;
    double newBalance;
    double minPayment;

    // initialize needed variables here
    keyboard = new Scanner(System.in);

    // make sure that the number entered is a double and if so read it in.
    // if not, use a default value of zero. No error message to the user.
    System.out.print("Type the balance: ");
    priorBalance = keyboard.nextDouble();

    if (keyboard.hasNextDouble() == true) {
        interestRate = 0.0;
    } else {
        interestRate = 0.02;
    }

    // make sure that the number entered is a double and if so read it in.
    // if not, use a default value of zero.  No error message to the user.
    System.out.print("\nType the new charge amount: ");
    addtlCharges = keyboard.nextDouble();

    // perform the calculation of interest, new balance and minimum payment
    newBalance = calculateInterest(priorBalance, addtlCharges);
    minPayment = calculateMinPayment(newBalance);

    // output the resulting statement (DO NOT CHANGE)
    System.out.println("\n");
    System.out.println("CS CARD International Statement");
    System.out.println("===============================");
    System.out.printf("Previous balance:   $%,8.2f\n", priorBalance);
    System.out.printf("Additional charges: $%,8.2f\n", addtlCharges);
    System.out.printf("Interest:           $%,8.2f\n\n", interest);
    System.out.printf("New balance:        $%,8.2f\n\n", newBalance);
    System.out.printf("Minimum payment:    $%,8.2f\n", minPayment);
}

/**
 * This method will take in the previous balance and
 * additional charges and compute the amount of interest.
 * If the priorBalance is 0 or less, the interest is 0.
 * If there was a priorBalance, the interest is 2% of the
 * priorBalance plus the additional charges.
 *
 * @param priorBalance balance before the new charges are added
 * @param addtlCharges charges added this month
 * @return amount of interest to charge
 */
public static double calculateInterest(double priorBalance,
                                       double addtlCharges) {
    // first create a stub and get the input and output working
    // then replace the stub later on with the calculation
    interest = (priorBalance + addtlCharges) * interestRate;
    return interest;
}

/**
 * This method will take in the previous balance and
 * additional charges and compute the minimum payment.
 *          $0.00          for new balance less than $0
 *      new balance        for new balance between $0 and $49.99 (inclusive)
 *         $50.00          for new balance between $50 and $300 (inclusive)
 * 20% of the new balance  for new balance over $300
 *
 * @param balance after interest and charges are added
 * @return minimum payment amount
 */
public static double calculateMinPayment(double balance) {
    // first create a stub and get the input and output working
    // then replace the stub later on with the calculation
    if (newBalance < 0) {
        balance = 0;
    }
    if (newBalance >= 0 && newBalance <= 49.99) {
        balance = newBalance;
    }
    if (newBalance >= 50 && newBalance <= 300) {
        balance = 50.0;
    }
    if (newBalance > 300) {
        balance = (newBalance * 0.2) + newBalance;
    }
    return balance;

}

}

Watch your scope - in your class you keep your data in two places - local variables, and static members. I cannot see any thought behind choosing where to put them - you pass some around in your method signatures, but then you use the others in the same methods... The most acute case is in calculateMinPayment where you pass the static member newBalance as a parameter and use it inside the method, while totally ignoring (and overriding) the parameter...

In the use-case above, in the name of encapsulation, I believe the correct way is to only keep the state as local variables, passing the needed data to the stateless helper methods, rather than keeping anything in the static state.

Comments - I personally think that comments are a code smell. When the code is clean and readable - comments are superfluous. You've got a lot of comments, and some are more confusing than helpful, since you put them in the wrong location:

// make sure that the number entered is a double and if so read it in.
// if not, use a default value of zero. No error message to the user.
System.out.print("Type the balance: ");
priorBalance = keyboard.nextDouble();

if (keyboard.hasNextDouble() == true) {
    interestRate = 0.0;
} else {
    interestRate = 0.02;
}

The above comments should be placed after setting priorBalance, since they describe interestRate... They are more confusing, since the code does not do what the comment say - it sets the interestRate at either 0.0 or 0.02, then totally ignores the given information, and tries to get nextDouble() even if the last statement returned false.

BTW - don't use if (keyboard.hasNextDouble() == true) - if (keyboard.hasNextDouble()) does exactly the same, and is more readable.

// make sure that the number entered is a double and if so read it in.
// if not, use a default value of zero.  No error message to the user.
System.out.print("\nType the new charge amount: ");
addtlCharges = keyboard.nextDouble();

Was that a result of an unfortunate copy+paste? Again - comment is not helpful, only confuses.

I also believe the prompt Type the new charge amount will appear only after the user entered it, since you check hasNextDouble() before you print the prompt.

// first create a stub and get the input and output working
// then replace the stub later on with the calculation

I don't even begin to understand what this means, and what it has to do with the code it describes... not the second time either...

Naming - give variables and methods names which imply their role in your code - what does this line do?

newBalance = calculateInterest(priorBalance, addtlCharges);

Calculates interest or the new balance?

Also, don't be lazy say - additionalCharges - there are no fines for long names.


Iterative Flood Fill implementation in C I am trying to implement an iterative version of flood fill in C:

#include<stdio.h>
int m,n;
void flood_fill(int [][n],int,int,int);
void print_wall(int [][n]);
int main(void)
{
    int i,j;
    scanf("%d%d",&m,&n);
    int arr[m][n];
    for(i=0;i<m;i++)
    {
        for(j=0;j<n;j++)
        {
            scanf("%d",&arr[i][j]);
        }
    }
    int x,y;
    printf("Enter the coordinates of the point to be repainted\n");
    scanf("%d%d",&x,&y);
    int new_col;
    printf("Enter new color to be painted");
    scanf("%d",&new_col);
    flood_fill(arr,x,y,new_col);
    print_wall(arr);
    return 0;
}

void flood_fill(int a[][n],int x,int y,int n)
{
    int i,j;
    for(i=x+1;i<m-1;i++)
    {
        for(j=y;j<n-1;j++)
        {

            if(a[i][j]==a[x][y])
            a[i][j]=n;

        }
    }
    for(i=x;i>0;i--)
    {
        for(j=y-1;j>0;j--)
        {

            if(a[i][j]==a[x][y])
            a[i][j]=n;

        }
    }
    for(i=x+1;i<m-1;i++)
    {
        for(j=y;j>0;j--)
        {

            if(a[i][j]==a[x][y])
            a[i][j]=n;

        }
    }
    for(i=x;i>0;i--)
    {
        for(j=y+1;j<n-1;j++)
        {
            if(a[i][j]==a[x][y])
            a[i][j]=n;

        }
    }
a[x][y]=n;
}

void print_wall(int a[][n])
{
    int i,j;
    for(i=0;i<m;i++)
    {
        for(j=0;j<n;j++)
        {
            printf("%d",a[i][j]);
        }
        printf("\n");
    }

}

Is my implementation correct? Is the stack/queue implementation better?

There is no need to have m and n as global variables. Create a structure to hold your data members together and pass a pointer to that structure to your functions.
Variable names are too short and not intuitive: m, n, arr, i, j, x, y etc. Use better names (ex: numberOfRows, numberOfCols etc) that explain what the variable is doing.
For readability reasons I would go with a recursive implementation. My second choice would be a stack based, if recursivity is not an option. And if performance would really be an important point of my implementation that I would do something similar but:

I would create a struct MatrixMap that would hold my matrix, number_of_rows and number_of_cols.
I would represent the position of an element from the MatrixMap using a structure MatrixMapCoordinate having two size_t values, Row and Column.
An element from the MatrixMap can be represented using a structure MatrixMapElement.
Accessing an element can be done with a function similar with:

const MatrixMapElement* ElementAt(const MatrixMap* fromMap, const MatrixMapCoordinate* atPosition);

I would move in my matrix using specialized functions such as MoveWest, MoveNorth, MoveEast and MoveSouth. You can even send flags to control the movement behavior: SkipAllEmptyCells, MoveOneCell etc.

Lastly, what I said at point 2 and 3 I would rewrite my algorithm as close as possible to the actual pseudocode.

//Read "elements" as color in the algorithm.
//  I decided to use element instead of color so you can use it in any kind of problem, regardless of what does your matrix contains.
void FloodFill(MatrixMap* targetMap, const MatrixMapCoordinate* startingCoordinate, const MatrixMapElement* targetElement, const MatrixMapElement* replacementElement)
{
    //If the starting coordinate is out of range we end the fill
    if (HasElementAt(targetMap, startingCoordinate))
        return;

    //Check if the element at the startingCoordinate is of the targetElements.
    if (ElementsAreEqual(ElementAt(targetMap, startingCoordinate), targetElement))
        return;

    //If both target and replacement are equal there is nothing to do.
    if (ElementsAreEqual(targetElement, replacementElement))
        return;

    SetElementValue(targetMap, startingCoordinate, replacementElement);

    FloodFill(targetMap, MoveWest(startingCoordinate), targetElement, replacementElement);
    FloodFill(targetMap, MoveNorth(startingCoordinate), targetElement, replacementElement);
    FloodFill(targetMap, MoveSouth(startingCoordinate), targetElement, replacementElement);
    FloodFill(targetMap, MoveEast(startingCoordinate), targetElement, replacementElement);
}

Note that there is no error or pointer check. I wrote the code just to give you an idea on how it should look from a readability point of view.

You can do a similar implementation for any algorithm you choose.

Because of the poor readability in your initial implementation I didn't spend time to validate your logic. Though I would be more than happy to do it, if you refactor your code using some of the suggestions I gave you.


Another Python Hangman I have finished my Python Hangman app with very basic code.

import random
import sys

# lets set some variables
wordList = [
"lion", "umbrella", "window", "computer", "glass", "juice", "chair", "desktop",
 "laptop", "dog", "cat", "lemon", "cabel", "mirror", "hat"
           ]

guess_word = []
secretWord = random.choice(wordList) # lets randomize single word from the list
length_word = len(secretWord)
alphabet = "abcdefghijklmnopqrstuvwxyz"
letter_storage = []

def beginning():
    print("Hello Mate!\n")

    while True:
        name = input("Please enter Your name\n").strip()

        if name == '':
            print("You can't do that! No blank lines")
        else:
            break

beginning()

def newFunc():
    print("Well, that's perfect moment to play some Hangman!\n")

    while True:
        gameChoice = input("Would You?\n").upper()

        if gameChoice == "YES" or gameChoice == "Y":
            break
        elif gameChoice == "NO" or gameChoice == "N":
            sys.exit("That's a shame! Have a nice day")
        else:
            print("Please Answer only Yes or No")
            continue

newFunc()

def change():

    for character in secretWord: # printing blanks for each letter in secret word
        guess_word.append("-")

    print("Ok, so the word You need to guess has", length_word, "characters")

    print("Be aware that You can enter only 1 letter from a-z\n\n")

    print(guess_word)

def guessing():
    guess_taken = 1

    while guess_taken < 10:

        guess = input("Pick a letter\n").lower()

        if not guess in alphabet: #checking input
            print("Enter a letter from a-z alphabet")
        elif guess in letter_storage: #checking if letter has been already used
            print("You have already guessed that letter!")
        else:

            letter_storage.append(guess)
            if guess in secretWord:
                print("You guessed correctly!")
                for x in range(0, length_word): #This Part I just don't get it
                    if secretWord[x] == guess:
                        guess_word[x] = guess
                        print(guess_word)

                if not '-' in guess_word:
                    print("You won!")
                    break
            else:
                print("The letter is not in the word. Try Again!")
                guess_taken += 1
                if guess_taken == 10:
                    print(" Sorry Mate, You lost :<! The secret word was",         secretWord)

change()
guessing()

print("Game Over!")

So, I have couple questions about my code:

I have started to lean some stuff about classes in Python and I thought "Hey, let's write this Hangman with Classes)" But after some coding I realised, i just made classes with 1 func and instead of func() I just called them class.func() and that's all. Is there any good reason why and how I should use classes in a Hangman game?
I know that code is very basic and at my level I shouldn't exactly mind effectiveness of the code but still - Which Part Should be improved ( not the code itself but my logic there)? I want to write in a "proper" way but my code still looks super basic.

Playability
I ran your game to test it out first, and noticed something odd when a letter is found twice in the same word, such as "glass" here:

Pick a letter
 A
You guessed correctly!
['-', '-', 'a', '-', '-']
Pick a letter
 s
You guessed correctly!
['-', '-', 'a', 's', '-']
['-', '-', 'a', 's', 's']

I'm not sure if that is by design, but as a player I find it odd that it's displayed twice. I found where it originated in the code:
for x in range(0, length_word): #This Part I just don't get it
    if secretWord[x] == guess:
        guess_word[x] = guess
        print(guess_word)

If you just unindent print(guess_word) by two levels, you'll avoid that behavior since it won't run in a loop:
for i in range(0, length_word):
    if secretWord[i] == guess:
        guess_word[i] = guess
print(guess_word)

Printing a raw array like ['-', '-', 'a', 's', 's'] is a bit confusing, at first I didn't know what it was for until I guessed one right and saw the results. So let's make it print something more friendly:
print("Word to guess: {0}".format(" ".join(guess_word)))

Pick a letter
 o
You guessed correctly!
Word to guess: - o -

Much better! But it will get a bit clunky to type all that each time, so let's make a utility function for it and we can use it for the print(guess_word) at the beginning of a game too.
def print_word_to_guess(letters):
    """Utility function to print the current word to guess"""
    print("Word to guess: {0}".format(" ".join(guess_word)))

Then we just print_word_to_guess(guess_word) whenever we need it. It would also be possible to make an optional different message but default to Word to guess:, but I'll leave that for you as a challenge.

The game also doesn't ever tell me anything about how many chances I have left, so I'm left guessing (literally) until I figure it out. That's very easy to make a small utility function to do:
def print_guesses_taken(current, total):
    """Prints how many chances the player has used"""
    print("You are on guess {0}/{1}.".format(current, total))

Then a few code additions:
def guessing():
    guess_taken = 1
    MAX_GUESS = 10
    print_guesses_taken(guess_taken, MAX_GUESS)

    while guess_taken < MAX_GUESS:

And:
for i in range(0, length_word):
    if secretWord[i] == guess:
        guess_word[i] = guess
print_word_to_guess(guess_word)
print_guesses_taken(guess_taken, MAX_GUESS)

Your list of words is pretty limited, for future consider perhaps looking for a text file online with a whole bunch of words and just read a random one from it, that would bring more variety!

Code Improvements
main function
You run all your game functions as soon as you make them. It would make more sense to put them inside the __main__ function:
if __name__ == "__main__":
    beginning()
    newFunc()
    change()
    guessing()

By the way, newFunc() doesn't work well as a name, as it says nothing about what it does. Something like ask_user_to_play() would be much better.

Constants naming
Python doesn't have real constants, but nevertheless, it's good practice to name variables that shouldn't change (by change I mean reassigned to a different values) in ALL_CAPS_WITH_UNDERSCORES. A simple find & replace in an IDE or text editor does the trick to fix your whole script.
GUESS_WORD = []
SECRET_WORD = random.choice(wordList) # lets randomize single word from the list
LENGTH_WORD = len(SECRET_WORD)
ALPHABET = "abcdefghijklmnopqrstuvwxyz"
letter_storage = []

Docstring
It's a good habit to add a docstring to all functions, classes, and modules, to describe for other programmers what the code in question is for. I've done that for the above 2 utility function.

Type hints
Since Python 3, you can now use type hints for your function and method signatures, and use static code analysis tools. This also makes the code easier to read for humans.

Improved code
Here is what I came up with,applying all of the above, and improving the spacing (yours had too many blank lines everywhere). There are likely other things that could be improved, but this should be a good start.
import random, sys
from typing import List

# TODO try to load these from a text file
WORD_LIST = [
"lion", "umbrella", "window", "computer", "glass", "juice", "chair", "desktop",
 "laptop", "dog", "cat", "lemon", "cabel", "mirror", "hat"
           ]

GUESS_WORD = []
SECRET_WORD = random.choice(WORD_LIST) # lets randomize single word from the list
LENGTH_WORD = len(SECRET_WORD)
ALPHABET = "abcdefghijklmnopqrstuvwxyz"
letter_storage = []

# Utility functions

def print_word_to_guess(letters: List) -> None:
    """Utility function to print the current word to guess"""
    print("Word to guess: {0}".format(" ".join(letters)))

def print_guesses_taken(current: int, total: int) -> None:
    """Prints how many chances the player has used"""
    print("You are on guess {0}/{1}.".format(current, total))

# Game functions

def beginning() -> None:
    """Starts the game"""
    print("Hello Mate!\n")
    while True:
        name = input("Please enter Your name\n").strip()
        if name == '':
            print("You can't do that! No blank lines")
        else:
            break

def ask_user_to_play() -> None:
    """Ask user if they want to play"""
    print("Well, that's perfect moment to play some Hangman!\n")
    while True:
        gameChoice = input("Would You?\n").upper()
        if gameChoice == "YES" or gameChoice == "Y":
            break
        elif gameChoice == "NO" or gameChoice == "N":
            sys.exit("That's a shame! Have a nice day")
        else:
            print("Please Answer only Yes or No")
            continue

def prepare_secret_word() -> None:
    """Prepare secret word and inform user of it"""
    for character in SECRET_WORD: # printing blanks for each letter in secret word
        GUESS_WORD.append("-")
    print("Ok, so the word You need to guess has", LENGTH_WORD, "characters")
    print("Be aware that You can enter only 1 letter from a-z\n\n")
    print_word_to_guess(GUESS_WORD)

def guessing() -> None:
    """
    Main game loop to have user guess letters
    and inform them of the results
    """
    guess_taken = 1
    MAX_GUESS = 10
    print_guesses_taken(guess_taken, MAX_GUESS)

    while guess_taken < MAX_GUESS:
        guess = input("Pick a letter\n").lower()
        if not guess in ALPHABET: #checking input
            print("Enter a letter from a-z ALPHABET")
        elif guess in letter_storage: #checking if letter has been already used
            print("You have already guessed that letter!")
        else:
            letter_storage.append(guess)
            if guess in SECRET_WORD:
                print("You guessed correctly!")
                for i in range(0, LENGTH_WORD):
                    if SECRET_WORD[i] == guess:
                        GUESS_WORD[i] = guess
                print_word_to_guess(GUESS_WORD)
                print_guesses_taken(guess_taken, MAX_GUESS)
                if not '-' in GUESS_WORD:
                    print("You won!")
                    print("Game Over!")
                    break
            else:
                print("The letter is not in the word. Try Again!")
                guess_taken += 1
                print_guesses_taken(guess_taken, MAX_GUESS)
                if guess_taken == 10:
                    print(" Sorry Mate, You lost :<! The secret word was {0}".format(SECRET_WORD))

if __name__ == "__main__":
    beginning()
    ask_user_to_play()
    prepare_secret_word()
    guessing()


Hacking a SecureString based on std::basic_string for C++ Inspired by reading "How-to write a password-safe class?", I tried some clever (or dumb-fool) hack to create a widely-useable secure string using the std::basic_string-template, which does not need to be explicitly securely erased itself.
At least gcc and clang seem not to choke on it (coliru):
#include <string>
namespace my_secure {
void SecureZeroMemory(void* p, std::size_t n) {
    for(volatile char* x = static_cast<char*>(p); n; --n)
        *x++ = 0;
}

// Minimal allocator zeroing on deallocation
template <typename T> struct secure_allocator {
    using value_type = T;

    secure_allocator() = default;
    template <class U> secure_allocator(const secure_allocator<U>&) {}

    T* allocate(std::size_t n) { return new T[n]; }
    void deallocate(T* p, std::size_t n) {
        SecureZeroMemory(p, n * sizeof *p);
        delete [] p;
    }
};

template <typename T, typename U>
inline bool operator== (const secure_allocator<T>&, const secure_allocator<U>&) {
    return true;
}
template <typename T, typename U>
inline bool operator!= (const secure_allocator<T>&, const secure_allocator<U>&) {
    return false;
}

using secure_string = std::basic_string<char, std::char_traits<char>,
    secure_allocator<char>>;
}

namespace std {
// Zero the strings own memory on destruction
template<> my_secure::secure_string::~basic_string() {
    using X =std::basic_string<char, std::char_traits<char>,
        my_secure::secure_allocator<float>>;
    ((X*)this)->~X();
    my_secure::SecureZeroMemory(this, sizeof *this);
}
}

And a short program using it to do nothing much:
//#include "my_secure.h"
using my_secure::secure_string;
#include <iostream>

int main() {
    secure_string s = "Hello World!";
    std::cout << s << '\n';
}

Some specific concerns:

How badly did I break the standard?

Does the fact that one of the template-arguments is my own type heal the fact that I added my own explicit specialization to ::std?
Are the two types actually guaranteed to be similar enough that my bait-and-switch in the destructor is ok?

Is there any actual implementation where the liberties I took with the standard will come back to haunt me?

Did I miss any place where I should zero memory after use? Or is there any chance that anything will slip by?

The only thing special about SecureZeroMemory (the Windows version) is that it uses volatile to prevent optimization. Therefore there's no reason to write your own. Just defer to standard algorithms:

std::fill_n((volatile char*)p, n*sizeof(T), 0);

You will probably want to call the global delete:

::operator delete [] p;

Add noexcept:

template <class U> secure_allocator(const secure_allocator<U>&) noexcept {}


Merge two sorted linkedList I'm learning Data structures and came across this problem - merging two sorted linked list. I have gone through the recursive solution, and it seems pretty straight forward. I wanted to try an iterative solution and came up with this.

private static Node mergeListsIterative(Node headA, Node headB) {

        if (headA == null) {
            return headB;
        }

        if (headB == null) {
            return headA;
        }

        Node head;

        if (headA.data < headB.data) {
            head = headA;
            headA = headA.next;
        } else {
            head = headB;
            headB = headB.next;
        }

        Node temp = head;
        while (headA != null && headB != null) {
            if (headA.data < headB.data) {
                temp.next = headA;
                headA = headA.next;
            } else {
                temp.next = headB;
                headB = headB.next;
            }
            temp = temp.next;
        }

        if (headA == null) {
            temp.next = headB;
        } else {
            temp.next = headA;
        }

        return head;
    }

I saw this post on SO which uses an iterative approach. I feel that my code is simpler. Please review my code and tell me if it can be simplified.

The name of the method is mergeLists, not mergeNodes. Even though you may think your method is simpler, using Nodes as parameters is misleading for the current method name. One way to address this issue is to rename the method as mergeListsUsingTheirHeads or, to be specific to the problem of merging sorted lists, mergeSortedListsUsingTheirHeads.

I have to ask: what made you think your code is simpler than that of the post you mentioned?


Determine if a file is either in JSON or XML format The purpose of this function is to determine if report_input_file is either in JSON format or XML format, this is what I came up with, just want to know if this is best practice / pythonic way of achieving this?

def parse_report_file(self, report_input_file):
    """ Checks if input report file is of type json or xml """
    parsed_report_file = None
    try:
        parsed_report_file = self._parse_json_file(report_input_file)
        self.is_json_report_file = True
    except Exception, e:
        parsed_report_file = self._parse_xml_file(report_input_file)
        self.is_json_report_file = False

    return parsed_report_file

Well, if you only need to distinguish between JSON and XML formats I'd keep it simple. If the encoding of that string is known to you (or is ASCII or UTF), then looking at the very first char in the file should be enough. Your code looks pythonic to me, but I'd rather do it this way:

def parse_report_file(report_input_file):
    with open(report_input_file) as unknown_file:
        c = unknown_file.read(1)
        if c != '<':
            return 'Is JSON'
        return 'Is XML'

While it is legal for JSON data structures to begin with null, true, false you can avoid those situations if you already know a bit about your data structures.

If you also need to distinguish between JSON, XML and other random file, you should simply strip any whitespace before you look at the "first" char.


Digit-by-digit addition using vectors I was asked to perform addition by using vectors, so I am wondering if the function I coded looks good. If not, how can I improve the quality of my code? Below, I provided .h and .cpp files. I need feedback on the string constructor and operator+ modules.

.h file

class ubigint {
   friend ostream& operator<< (ostream&, const ubigint&);
   private:
      /*using unumber = unsigned long;
      unumber uvalue {};
      */
      using udigit_t = unsigned char;
      using ubigvalue_t = vector<udigit_t>;
      ubigvalue_t ubig_value;

   public:
      void multiply_by_2();
      void divide_by_2();

      ubigint() = default; // Need default ctor as well.
      ubigint (unsigned long);
      ubigint (const string&);

      ubigint operator+ (const ubigint&) const;
      ubigint operator- (const ubigint&) const;
      ubigint operator* (const ubigint&) const;
      ubigint operator/ (const ubigint&) const;
      ubigint operator% (const ubigint&) const;

      bool operator== (const ubigint&) const;
      bool operator<  (const ubigint&) const;
};

Currently, I am just done with the constructor and operator+. I feel like my function needs a little improvement since I seem to be repetitive in some places; however, a lot of people may say that the exhaustive creation of modules may lead to poor performance. I just tried to keep my code clean, but I don't know if the quality is good. I neither have checked if the code runs since I have to do other stuff in order to do so.
.cpp file

ubigint::ubigint (const string& that){
   DEBUGF ('~', "that = \"" << that << "\"");
   for (char digit: that) {
      if (not isdigit (digit)) {
         throw invalid_argument ("ubigint::ubigint(" + that + ")");
      }
   }

   for(string::reverse_iterator rit = that.str().rbegin(); rit != that.str().rend(); rit++) // iterate the string from end to start and store it in the vector
    {
        ubig_value.push_back(*rit); // push the character
    }

}

 ubigint ubigint::operator+ (const ubigint& that) const {  //modify to handle vector wise addition

    int carry = 0;  // current carrier
    int counter = 0;
    int pairAdditionResult = 0;
    int vectorValue = 0;
    udigit_t vectorCharValue = '0';
    ubigvalue_t result;

    do{
        pairAdditionResult = (ubig_value[counter] -'0') + (that[counter] - '0') + carry;
        carry = pairAdditionResult / 10;
        vectorValue = pairAdditionResult % 10;
        vectorCharValue = vectorValue + '0';
        result.push_back(vectorCharValue);
        counter++;

    }while(counter != ubig_value.size() && counter != that.size())

    if(ubig_value.size() != counter )  // if the size of the vectors differ
    {
        do{
          pairAdditionResult = (ubig_value[counter] - '0') + carry;
          carry = pairAdditionResult / 10;
          vectorValue = pairAdditionResult % 10;
          vectorCharValue = vectorValue  + '0';
          result.push_back(vectorCharValue);
          counter++;

        }while(counter != ubig_value.size())

    }
    else if (that.size() != counter)
    {
        do{
            pairAdditionResult =(that[counter] - '0') + carry;
            carry = pairAdditionResult / 10;
            vectorValue = pairAdditionResult % 10;
            vectorCharValue = vectorValue + '0';
            result.push_back(vectorCharValue);
            counter++;
        }while(counter != that.size(counter != that.size()))

    }

    if(carry != 0)  // for the last digit of the result
    {
        result.push_back(carry + '0');
    }

    int resultVectorSize = result.size();
    string resultVectorString;  // used for passing the number as a string in the returned object
    while(resultVectorSize != 0)
    {
        resultVectorString.push_back(result[resultVectorSize-1])
        resultVectorSize--;
    }

   return ubigint (resultVectorString);
}

I'm not sure whether you used using namespace std; in the header, or only committed the lesser evil of needlessly importing many names into the global namespace in a header. Anyway, as you are implementing some new library-code, open your own namespace and confine your code to it.
Please read Why is “using namespace std;” considered bad practice?

Let's take a look at your data-structure:

You are using a std::vector, so you have its re-allocation-behavior of roughly doubling the space on demand, and keeping the maximum ever reserved. Not a bad choice.
You store your number as single digit characters. That's workable, but you should really think about storing 0 as 0 not '0'. It makes math easier. Still, I'll keep with that.
Binary operators should, for those where the language allows it, be free functions or at least friend-functions to reduce coupling and enable conversion on the left hand side.
That holds even if you later add overloads to remove those conversions for efficiency.
If you provide some binary operator, it's convenient and customary to also provide the corresponding compound-assignment-operator.

And now your interface:

A conversion (single-argument-ctor or operator Type) should only be implicit if it is lossless and is expected to succeed for all valid inputs.
But "A" is a valid string but will not be accepted.
Use std::string_view instead of constant reference to std::string as input. Thus the caller isn't forced to use a std::string with the possible dynamic allocation (and copy) that entails.

Let's take a look at your ctor:

How does DEBUGF work? Maybe it's a debug-tracing-macro, but what's the first argument for then? Oh well, it's not that important.
You know exactly how long the input is, so why don't you .reserve() space for it to avoid reallocations?
You know calling std::isdigit() with a negative value != EOF is undefined behavior? Cast to unsigned char first!
You optimized your ctor for bad input, as you do a first pass solely to verify the input, and then a second pass to finally store it. Combine the passes into one.
You should probably eliminate leading zeroes. That also makes everything easier.

ubigint::ubigint(std::string_view that) {
    auto pos = that.find_first_not_of('0');
    if(pos != that.npos)
        that = that.substr(pos);

    ubig_value.reserve(that.size());

    for(auto it = rbegin(that); it != rend(that); ++it) {
        if(!isdigit((unsigned char)*it))
            throw invalid_argument ("ubigint::ubigint(" + that + ")");
        ubig_value.push_back((unsigned char)*it);
    }
}

Now, your addition:

While it is often recommended to implement operator+ as a free function in terms of operator+= to avoid repetition, your class doesn't qualify, as it would result in (maybe severe) loss of efficiency. You might want to do it the other way around instead.
Still, that doesn't mean that your try is good. Your first loop repeatedly tests both inputs lengths. Why don't you use std::max(a, b)?
You are using / and % to calculate the carry and the value. While it works, it's highly inefficient. You know the carry is at most one, so use an if-statement and subtraction.
Point 2 comes back to bite you there. See how biasing the numbers with '0' creeps everywhere into the calculations?
The following if-else if-statement is just painful. You are doing the same for both cases, just with a different input-variable. Merge that.
The end really gets me scratching my head. Why do you convert the perfectly usable ubigint into a std::string and then back on returning?
I haven't seen .size() and operator[] in your interface, but adding them is a good idea, so I pretend you already did so. You might want to overhaul your data-structure first though, to be 0 instead of '0'-based.

ubigint operator+ (const ubigint& a, const ubigint& b) {
    if(a.size() > b.size())
        return b + a;

    bool carry = false;
    ubigint result;
    result.reserve(b.size() + 1);
    std::size_t i = 0;
    for(; i < a.size(); ++i) {
        auto c = a[i] + b[i] + carry - '0';
        if(c <= '9') {
            carry = false;
        } else {
            c -= 10;
            carry = true;
        }
        result.push_back(c);
    }

    while(carry && i < b.size()) {
        auto c = b[i] + carry;
        if(c <= '9')
            carry = false;
        else
            c -= 10;
        result.ubig_value.push_back(c);
    }

    if(!carry)
        std::copy(b.begin() + i, b.end(), std::back_inserter(result));
    else
        result.ubig_value.push_back(c);
    return result;
}


How can I make this C++ code to obtain passwords more efficient? The comments in the code kind of explain it. Backspace characters only move cursor back and don't erase output so I need to overwrite them with ' ' to delete them.
#include <iostream>
#include <string>
using namespace std;

const string PASSWORD = "PASSWORD";
int main(int argc, char** argv) {
string userkey = "";
char c;
int keystrokes = 0;

cout<<"Enter passkey: ";
while(true){
    if(_kbhit()){
        c=_getche();
        if(c=='\r'){//check if user hit enter, _getche() records enter as \r
            cout<<'\n';
            break;
        }
        else if(c =='\b'){//check if user typed backslash, overwrite character there
            cout<<' ';
            if(keystrokes==1){ //makes sure userkey is cleared
                userkey = "";
                keystrokes--;
                cout<<'\b';
            }
            else if(keystrokes!=0){
                userkey = userkey.substr(0,userkey.length()-1);//cuts out last char of userkey
                keystrokes--;
                cout<<'\b';
            }
        }
        else if (c!='\b'){
            cout<<"\b*";//replaces enter char with'*'
            keystrokes++;
            userkey+=c;
        }
    }
}
//cout<<userkey;
if (userkey!=PASSWORD)
    return 0;

Regarding your program's efficiency, remember that premature optimisation is the root of all evil. Looking at your program, I can tell that your program will be IO bound - meaning that the computer will spend longer waiting for the user to input the data than actually executing your program. Moreover, functions like std::cout which deal with output, include inevitable slow system calls, which will be far costly in time than your algorithm. So performance should not be a concern here ! :)
The only thing I would say about your program efficiency is const string PASSWORD = "PASSWORD"; which will use unnecessary copying and heap allocation. It is better to preserve the original type of the string like so:
const auto PASSWORD = "PASSWORD";  (Here auto (assuming c++11) deduces the type for you, but the real type of the string is const char* if you want to know)
CODE REVIEW
These are the style/ readability points I would make on your code.

Use proper indentation (this might due to copy pasting on codereview so I don't blame you for that)
Do not use using namespace std - this will lead to naming collisions and all sort of horrible errors. Better to use the std:: every-time instead.
You might consider to put the variable PASSWORD inside main as a static const variable, this will limit the visibility of PASSWORD.
Use a better variable name than c.
If your program gets bigger, consider wrapping your algorithm inside a function, this might help to make the main function a bit neater.

That's all! Well done, I would say generally this is very well written, readable c++ code.


Importing data into Excel Is there an easier way of importing data into an Excel array or other data structure? I've tried researching collections but I have found the documentation hard to comprehend.

MSDN

The code I have opens a select file and searches for the column header and then loops through each row storing the data according to header and row variables. I've done this method for many macros in the past but now I am dealing with many many columns and I'm looking for a more advanced way.

Sub Import_NAVRec()

MyPath = Range("b2")                                'Defines cell that contains path to source file
Workbooks.Open (MyPath)                             'Opens file
Set tempbook = ActiveWorkbook                       'Names workbook
LR = Range("A65000").End(xlUp).Row                  'finds last row in sourcefile

ReDim aNavRec(1 To LR, 1 To 4)                      'Defines NAV Rec array
nRow = 0

 cName = "Accounting Basis"
 CA = Cells.Find(What:=UCase(cName), After:=ActiveCell, LookIn:=xlFormulas, LookAt:=xlWhole, SearchOrder:=xlByRows, SearchDirection:=xlNext).Column
 cName = "Accounting Date"
 cB = Cells.Find(What:=UCase(cName), After:=ActiveCell, LookIn:=xlFormulas, LookAt:=xlWhole, SearchOrder:=xlByRows, SearchDirection:=xlNext).Column
 cName = "Asset Currency"
 cC = Cells.Find(What:=UCase(cName), After:=ActiveCell, LookIn:=xlFormulas, LookAt:=xlWhole, SearchOrder:=xlByRows, SearchDirection:=xlNext).Column

     For r = 2 To LR
        'If Cells(r, cB) = "Trading Gain Loss" Then
         nRow = nRow + 1
         aNavRec(nRow, 1) = Cells(r, CA) 'Fund Number
         aNavRec(nRow, 2) = Cells(r, cB) 'Ledger
         aNavRec(nRow, 3) = Cells(r, cC) 'Balance change
        'End If

     Next r

tempbook.Close
End Sub

Sub Print_output()

Sheets("Output").Select
Set Destination = Range("a2")
Destination.Resize(UBound(aNavRec, 1) + 1, UBound(aNavRec, 2)).Value = aNavRec

End Sub

I propose the following refactoring based on:

use of arrays to range and viceversa

though tempered by the fact that you have to deal with possible non contiguous columns range
use of fully qualified range references

to both avoid Select/Activate stuff and have full control of which range/worksheet/workbook you're dealing with
use of Option Explicit statement

to force explicit declaration of type for ALL variables used in the code

this extra work earns you back with a lot more control of what you're actually doing and saves you a lot of time in both code debugging and maintenance
robust function to return a "valid" workbook

see GetWorkBook() function

`

Option Explicit

Sub Import_NAVRec()
    Dim tempbook As Workbook
    Dim LR As Long, LC As Long, c As Long, nCols As Long
    Dim aNavRec() As Variant                            'declare NAV Rec array. it'll be an array of arrays
    Dim headers As String                               'declare the variable where you'll put headers of interest

    Set tempbook = GetWorkBook(ActiveSheet.Range("b2")) 'try setting a workbook whose path is in cell "B2" of ActiveSheet

    If tempbook Is Nothing Then Exit Sub                'exit sub if no valid workbook is returned

    headers = "|Accounting Basis|Accounting Date|Asset Currency|"   ' set your headers delimited by a "|"
    ReDim aNavRec(1 To UBound(Split(headers, "|")) - 1)             ' dim aNavRec to number of headers you want to grab columns of
    With tempbook.ActiveSheet
        LR = .Cells(.Rows.Count, "A").End(xlUp).Row                 'find last row in sourcefile column A
        LC = .Cells(1, .Columns.Count).End(xlToLeft).Column         'find last column in sourcefile headers

        For c = 1 To LC 'loop through every column
            If InStr(headers, "|" & .Cells(1, c) & "|") <> 0 Then   'if current column header is bewteen the wanted ones
                nCols = nCols + 1                                   'update the number of found columns
                aNavRec(nCols) = Application.Transpose(.Range(.Cells(1, c), .Cells(LR, c))) 'store the found column in the variant element in the current index (nCols) of the array. you have to transpose a "column" range to fit into a "horizontal" variant array
            End If
        Next c
        ReDim Preserve aNavRec(1 To nCols)              ' redim aNavRec to actual number of headers found nt to grab columns of
    End With
    tempbook.Close (False)

    Print_output Worksheets("Output").Range("a2"), aNavRec 'after closing 'tempbook' the active workbook (and worksheet) is the one we started with

End Sub

Sub Print_output(iniRng As Range, arrays() As Variant)
    Dim i As Long
    With iniRng
        For i = 1 To UBound(arrays)
            iniRng.Offset(, i - 1).Resize(UBound(arrays) + 1).Value = Application.Transpose(arrays(i)) ' you have to transpose back the array to fit it into a "column" range
        Next i
        .CurrentRegion.EntireColumn.AutoFit
    End With
End Sub

Function GetWorkBook(path As String) As Workbook
    ' returns the workbook corresponding to path
    ' checks whether a workbook with the given name and path is already open
    ' if an already open workbook has the same name but different path:
    ' -  if it is NOT the active one -> it'llbe closed and the requested one will be opened
    ' -  if it IS the active one -> no action will be taken
    '
    ' requires reference to "Microsoft Scripting RuntTime" library

    Dim fso As New FileSystemObject
    Dim f As File
    Dim fName As String

    If Not fso.GetExtensionName(path) Like "xls*" Then Exit Function 'exit if it's not a valid excel file

    If Not fso.FileExists(path) Then Exit Function 'exit if there's no such file

    Set f = fso.GetFile(path) 'get the file corresponding to the now validated path

    fName = fso.GetFileName(path)

    Set GetWorkBook = TrySetWorkbook(fName) 'check whether that workbook is already open
    If GetWorkBook Is Nothing Then 'if no...
        Set GetWorkBook = Workbooks.Open(path) '... open it!
    Else 'if yes...
        If GetWorkBook.FullName <> f.path Then '... and it has a different path from the requested workbook one ...
            ' ... then you should close the already open workbook with the same of the requested one before opening this latter
            If ActiveSheet.Parent.name = fName Then 'if the workbook to close is the active one (!)...
                MsgBox "workbook:" & vbCrLf & vbCrLf & "'" & ActiveSheet.Parent.FullName & "'" _
                       & vbCrLf & vbCrLf & "should be closed to open woorkbook:" & vbCrLf & vbCrLf & "'" & f.path _
                       & vbCrLf & vbCrLf & vbCrLf & "but the former is the active workbook, so no action will be taken", vbInformation
                Set GetWorkBook = Nothing ' set the return workbook to Nothing so as to have calling sub skip the passed path
            Else 'otherwise...
                Workbooks(fName).Close True '... close (saving changes) the already open one...
                Set GetWorkBook = Workbooks.Open(path) '<~~ ... and finally open the requested one
            End If
        End If
    End If

End Function

Function TrySetWorkbook(path As String) As Workbook
    On Error Resume Next
    Set TrySetWorkbook = Workbooks(path)
End Function