Discount card reduction calculator I have started programming a couple of months ago and have recently applied for an internship. One of the assignments was to create a console application, the assignment can be seen here - > HERE

My implementation is this:

I have created an abstract class called DiscountCard which holds all the info, getters and setters, and has one abstract method of calculateDiscountRate.

public abstract class DiscountCard {

    private String owner;
    private int turnover;
    private int purchaseValue;
    private double discountRate;

    public DiscountCard(int turnOver, int purchaseValue){

        setTurnover(turnOver);
        setPurchaseValue(purchaseValue);

    }

    abstract void calculateDiscountRate();

    public String getOwner() {
        return owner;
    }

    public void setOwner(String owner) {
        this.owner = owner;
    }

    public int getTurnover() {
        return turnover;
    }

    public void setTurnover(int turnOver) {

        if(turnOver>=0) {
            this.turnover = turnOver;
        }else{
            System.out.println("Turnover can't be less than 0.");
        }

    }

    public int getPurchaseValue() {
        return purchaseValue;
    }

    public void setPurchaseValue(int purchaseValue) {

        if(purchaseValue>=0) {
            this.purchaseValue = purchaseValue;
        }else{
            System.out.println("Purchase value can't be less than 0.");
        }
    }

    public void setDiscountRate(double discountRate) {
        this.discountRate = discountRate;
    }

    public double getDiscountRate() {
        return discountRate;
    }
}

Then, there are 3 Card classes (Bronze,Silver,Gold) which implement the above class and override its abstract method:

 package com.test.DiscountCards;

    public class GoldDiscountCard extends DiscountCard {

        public GoldDiscountCard(int turnOver, int purchaseValue){

            super(turnOver,purchaseValue);
            calculateDiscountRate();
        }

        @Override
        void calculateDiscountRate() {

            setDiscountRate(2);

            setDiscountRate(getTurnover()/100 + getDiscountRate());

            if(getDiscountRate()>10){
                setDiscountRate(10);
            }

        }

    }

Next is the StoreCalculator Class which takes an instance of DiscountCard class and calculates the discount and total.

package com.test;

import com.practice.DiscountCards.DiscountCard;

public class StoreCalculator {

    private DiscountCard discountCard;

    public StoreCalculator(DiscountCard discountCard){
        this.discountCard = discountCard;
    }

    public double calculateDiscount(){
        return discountCard.getPurchaseValue() * discountCard.getDiscountRate()/100;
    }

    public double calculateTotal(){
        return discountCard.getPurchaseValue() - calculateDiscount();
    }
}

And at last i have a class that takes the info from DiscountCard class and Calculator class and prints it to a console.

import com.test.DiscountCards.DiscountCard;

public class PayDesk {

    public static void print(DiscountCard discountCard){

        StoreCalculator calculator = new StoreCalculator(discountCard);

        System.out.println("Purchase value: $"+ discountCard.getPurchaseValue() );
        System.out.println("Discount rate: "+ discountCard.getDiscountRate()+"%" );
        System.out.println("Discount: $"+ calculator.calculateDiscount() );
        System.out.println("Total: $"+ calculator.calculateTotal());

    }
}

My question is if this code implements OOP and is there something i should improve. I have send this code for a review to a company, i just want to know if there can be an improvement so i can learn more.

The OO part seems to be OK when it comes to the type inheritance and the choice of an abstract class.

Code review.

For the abstract class

public DiscountCard(int turnOver, int purchaseValue){

This is a public constructor for a class that cannot be instantiated, consider protected instead. It also contains a turnOver, but that's specific to an owner it seems to me. So that doesn't make sense without the owner string. Similarly, I don't think the card is specific to one particular purchase. The card is applied on a particular purchase.

    setTurnover(turnOver);
    setPurchaseValue(purchaseValue);

Generally you'd set the fields directly, instead of calling the public methods. Calling methods can be dangerous, especially if they can be overridden.

abstract void calculateDiscountRate();

For what? That depends on purchaseValue but that should not depend on any class fields, as already established.

public String getOwner() {
    return owner;
}

Here you make a classic mistake: never leave your objects in an invalid state. As the owner may not be set, this method could return null.

public void setOwner(String owner) {
    this.owner = owner;
}

No. If the holder of the card can change then this would be in the model, and I don't see that. Generally cards are personalized and are therefore not transferable (at least from the payment desk's point of view).

public void setTurnover(int turnOver) {
    if(turnOver>=0) {
        this.turnover = turnOver;
    }else{
        System.out.println("Turnover can't be less than 0.");
    }
}

The turnover may actually be negative if the customer decided to return. In this kind of situations it is important to perform a reality check: what could happen. Furthermore, I guess that this should be called after each period of time (it's for the "previous month" according to the instructions. So call it, say, setTurnoverOfLastMonth() or something similar.

public void setDiscountRate(double discountRate) {
    this.discountRate = discountRate;
}

Is that really a public call, you override the discount rate? Seems to me that you'd have to switch cards for that.

For the gold card:

calculateDiscountRate();

By now you should have noticed that there are more rates possible, you are still trying to get along with one.

setDiscountRate(2);

setDiscountRate(getTurnover()/100 + getDiscountRate());

And you aren't just overwriting the single discount rate in the second call?

if(getDiscountRate()>10){
    setDiscountRate(10);
}

Leukoplast patching ain't gonna help, son.

For the store calculator

Where are the purchases? In the card?

I'd expect something like:

public Payment calculateResult(double purchaseValue);

Where the Payment contains all the details (card used, discount percentage, discount value and of course the final price).

Note too that the examples have a purchase value with .00 behind it. Somehow that indicates to me that every cent is appreciated (and you store as an int - which is actually fine if it means cents).

For the PayDesk

Just prints out stuff. Maybe correct, maybe it isn't, but just printing the bill is not what I was expecting. Probably can be merged with the StoreCalculator although I like the fact that you've created a separate calculator. That would certainly be a good idea for a more complicated PayDesk class.

Beware: it says in your requirements: "create instances with sample data as shown in Example outputs section;"

Now I don't personally think that the purchase should ever be part of the card data, but if it is stated this way I'd almost include it myself as a field.

Very non-nerdy of me, but I'd include a welcoming message for the card holder (Thank you for your purchase, ). Your GOLD CARD discount is: ... Etc... This would show your employer that you can act outside what is basic development.

And probably I'd also welcome non-card holders to make a purchase.


Homescreen Widget Service I have implemented changes from a question I asked on this widget here:

Android widget code

I've added and changed a few things, so I would love some more coding advice!

public class SacWidgetService extends Service {
    private static final String TAG = "SAC WIDGET SERVICE";
    AQuery aq;
    private String roseXML = "sac-full.xml";
    private String roseUrl;
    private AppWidgetManager appWidgetManager;
    private int[] allWidgetIds;
    private File ext;
    private File file;

    @Override
    public void onStart(Intent intent, int startId) {

        aq = new AQuery(this);
        // Create some random data
        appWidgetManager = AppWidgetManager.getInstance(this
                    .getApplicationContext());

        allWidgetIds = intent.getIntArrayExtra(AppWidgetManager.EXTRA_APPWIDGET_IDS);

        // get external directory and put the file in there.
        ext = Environment.getExternalStorageDirectory();
        file = new File(ext, "avalancheconditionsreport/sac/dangerrose.png");

        // parse xml and get the rose
        xml_ajax();

        stopSelf();

        super.onStart(intent, startId);
    }

    @Override
    public IBinder onBind(Intent intent) {
        return null;
    }

    public void xml_ajax() {
        aq.ajax(roseXML, XmlDom.class, this, "sacCb");
    }

    public void sacCb(String url, XmlDom xml, AjaxStatus status) {
        XmlDom r = xml.tag("reportrose");
        roseUrl = r.tag("img").attr("src").trim();

        File ext = Environment.getExternalStorageDirectory();
        File target = new File(ext,
                "avalancheconditionsreport/sac/dangerrose.png");

        aq.download(roseUrl, target, new AjaxCallback<File>() {
            public void callback(String url, File file, AjaxStatus status) {
                String s = status.getMessage();
                int i = status.getCode();
                String e = s + " | Status Code: " + i;

                if (file != null) {
                    getInfo();
                } else {
                    Log.e("ACR SAC Widget Error", e);
                }
            }

        });
    }

    private Bitmap getRose() {
        Bitmap bitmap = null;
        try {
            FileInputStream input = new FileInputStream(file);
            bitmap = BitmapFactory.decodeStream(input);
            try {
                input.close();
            } catch (IOException e) {
                // TODO Auto-generated catch block
                Log.e(TAG, e.getMessage());
            }
        } catch (FileNotFoundException e) {
            Log.e(TAG, e.getMessage());
        }
        // Toast.makeText(this, "Sierra Avalanche Center Danger Rose",
        // Toast.LENGTH_LONG).show();
        return bitmap;

    }

    private void getInfo() {
        // TODO Auto-generated method stub
        Date dlm = new Date(file.lastModified());
        String lm = dlm.toLocaleString();

        for (int appWidgetId : allWidgetIds) {
            RemoteViews views = new RemoteViews(this.getApplicationContext()
                    .getPackageName(), R.layout.sac_widget_layout);

            Intent clickIntent = new Intent(this.getApplicationContext(),
                    SACActivity.class);
            PendingIntent clickapp = PendingIntent.getActivity(
                        this.getApplicationContext(), 0, clickIntent,
                        PendingIntent.FLAG_UPDATE_CURRENT);

            views.setOnClickPendingIntent(R.id.ivdangerrose, clickapp);
            views.setTextViewText(R.id.tvupdated, "Updated: " + lm);
            views.setImageViewBitmap(R.id.ivdangerrose, getRose());
            appWidgetManager.updateAppWidget(appWidgetId, views);
        }
    }
}

try {
    FileInputStream input = new FileInputStream(file);
    bitmap = BitmapFactory.decodeStream(input);
    try {
        input.close();
    } catch (IOException e) {
        // TODO Auto-generated catch block
        Log.e(TAG, e.getMessage());
    }
} catch (FileNotFoundException e) {
    Log.e(TAG, e.getMessage());
}

If decodeStream throws an exception the stream won't be closed. A proper solution is the following:

try {
    FileInputStream input = new FileInputStream(file);
    try {
        bitmap = BitmapFactory.decodeStream(input);
    } finally {
        input.close();
    }
} catch (FileNotFoundException e) {
    Log.e(TAG, e.getMessage());
} catch (IOException e) {
    Log.e(TAG, e.getMessage());
}

Or this:

try {
    FileInputStream input = new FileInputStream(file);
    try {
        bitmap = BitmapFactory.decodeStream(input);
    } finally {
        input.close();
    }
} catch (IOException e) {
    Log.e(TAG, e.getMessage());
}

(Since FileNotFoundException is a subclass of IOException).

AQuery aq;

I guess this field could be private. (Should I always use the private access modifier for class fields?; Item 13 of Effective Java 2nd Edition: Minimize the accessibility of classes and members.)

private String roseUrl;
...
private File ext;

These field is used only in one method (onStart and sacCb). They could be local variables instead. (Effective Java, Second Edition, Item 45: Minimize the scope of local variables)

public void xml_ajax() {

This method name does not follow the usual camelCase conventions and it should be a verb. From Code Conventions for the Java Programming Language:

  Methods should be verbs, in mixed case with the first letter
  lowercase, with the first letter of each internal word capitalized.

I'd use longer and more descriptive variable names (which explains their purpose) for better readability than this ones:

XmlDom r = xml.tag("reportrose");
...
String s = status.getMessage();
int i = status.getCode();
String e = s + " | Status Code: " + i;
...
String lm = dlm.toLocaleString();

Longer names would make the code more readable since readers don't have to decode the abbreviations every time and when they write/maintain the code don't have to guess which abbreviation the author uses.

private Bitmap getRose() {
    Bitmap bitmap = null;
    try {
        FileInputStream input = new FileInputStream(file);
        try {
            bitmap = BitmapFactory.decodeStream(input);
        } finally {
            input.close();
        }
    } catch (IOException e) {
        Log.e(TAG, e.getMessage());
    }
    return bitmap;
}

You can get rid of the bitmap variable here if you return immediately:

private Bitmap getRose() {
    try {
        FileInputStream input = new FileInputStream(file);
        try {
            return BitmapFactory.decodeStream(input);
        } finally {
            input.close();
        }
    } catch (IOException e) {
        Log.e(TAG, e.getMessage());
    }
    return null;
}

See Guideline 1-2: Release resources in all cases in Secure Coding Guidelines for the Java Programming Language

String lm = dlm.toLocaleString();

This method is deprecated, you should use something else.

// TODO Auto-generated catch block

Comments like this doesn't seem like professional work. Solve the issues and remove the comments.


Knowing which way to iterate the for loop C I want the code to automatically know which way to iterate through the loop without manually adding the '>' or '<' sign.
I am using the logic that if the setup part of the loop is smaller than the value in the  test expression, then the sign used will '<' and will be '>' vice versa.

The current way I have gone around this is by wrapping the for loop in an if statement, however this looks incredibly bulky and I feel there must be a better way.

The following is the code I am currently using:

i = -1;

if ((i*i*i)<(i * i * (i + i + i + i + i))){
    for (integer = i * i * i; integer < i * i * (i + i + i + i + i);integer += i * i * i * i * i) {
        printf("%d",integer);
        printf("\n");
    }
}
else {
    for (integer = i * i * i; integer > i * i * (i + i + i + i + i);integer += i * i * i * i * i) {
        printf("%d",integer);
        printf("\n");
    }
}

And here is an example which looks simpler:

if (-1<-5){
for (integer = -1; integer < -5;integer += -1) {
    printf("%d",integer);
    printf("\n");
}
}

else{
    for (integer = -1; integer > -5;integer += -1) {
        printf("%d",integer);
        printf("\n");
    }
}

You have a really complicated example with variables, and a simpler one with literals. The literal one is only going to use one branch. So let's do a simple one with variables. Let's say you want to iterate from a to b, but you don't know which is larger. Well, first of all, unless I'm missing something, if a>b, then you'll want integer -= increment in the second branch, while your code has integer += increment in both. So before your for-loop, define a variable direction equal to the sign of b-a (that is, if b-a >0 then direction = 1, if b-a<0 then direction = -1). Then do:

for(integer = a; (b-integer)*direction > 0; integer += direction*increment)


Algorithm to remove duplicates array leaving at most 2 elements My interview question was that I need to return the length of an array that removed duplicates but we can leave at most 2 duplicates.

For example, [1, 1, 1, 2, 2, 3] the new array would be [1, 1, 2, 2, 3]. So the new length would be 5. I came up with an algorithm with \$O(2n)\$ I believe.

def removeDuplicates(nums):
    if nums is None:
        return 0

    if len(nums) == 0:
        return 0

    if len(nums) == 1:
        return 1

    new_array = {}
    for num in nums:
        new_array[num] = new_array.get(num, 0) + 1

    new_length = 0
    for key in new_array:
        if new_array[key] > 2:
            new_length = new_length + 2
        else:
            new_length = new_length + new_array[key]

    return new_length

new_length = removeDuplicates([1, 1, 1, 2, 2, 3])
assert new_length == 5

Can I improve this code to be any faster?

You're just reimplementing collections.Counter.

def remove_duplicates(lst):
    return sum(min(x, 2) for x in collections.Counter(lst).values())

By the way, \$O(2n)\$ is equivalent \$O(n)\$. However, the bound is actually tighter: \$O(n + m)\$, where m is the number of distinct elements.

This is, however, an average bound, because it relies on a dict. If the worst case hash collisions happen, you get \$O(m(n+m))\$


Portfolio landing page I'm trying to grasp this whole MVC design pattern idea but I'm in the dark if I'm doing it correct. This is where I need some feedback on what I've done so far. Here's what I'm wondering:

Does the hideMenu and showMenu fall under the view or controller?
Have I put viewNewPage in the correct block?
How can I make the animation of the menu boxes less repetetive but still allow jQuery to wait until all of the animations have been run before updating the view with the new page? This should probably be done via a class containing CSS3 animations but I'm really unsure on how to rewrite it.
Should I bind elements to functions in the controller or model?
Have I made any other blunders here when trying to follow the MVC pattern?

Simple markup:

Portfolio

Biography

Blog

Logo here

Contact

Snippets

jQuery:

$(document).ready(function() {

var model = {
    elements: {
        wrapper: $('main.content'),
        portfolio: $('#portfolio'),
        biography: $('#biography'),
        blog: $('#blog'),
        logo: $('#logo'),
        contact: $('#contact'),
        snippets: $('#snippets')
    },
    getPage: function(href) {

        return $.ajax({
            url: '../includes/pages/' + href + '.php',
            type: 'post'
        });
    }
}

var view = {
    viewNewPage: function(promise) {

        promise.done(function(data) {
            model.elements.wrapper.append(data).hide().fadeIn();
        });
    }
}

var ctrl = {
    hideMenu: function() {

        var box = $(this);
        var href = box.data('href');

        model.elements.portfolio.animate({top: '-50%'}, 500);
        model.elements.biography.animate({top: '-70%'}, 500);
        model.elements.blog.animate({left: '-45%'}, 500);
        model.elements.logo.fadeOut('slow');
        model.elements.contact.animate({bottom: '-50%'}, 500);
        model.elements.snippets.animate({right: '-55%'}, 500, function() {
            var promise = model.getPage(href);
            view.viewNewPage(promise);
        });
    },
    setElementBindings: function() {
        model.elements.wrapper.on('click', '.hoverable', ctrl.hideMenu);
        model.elements.wrapper.on('click', '.home', ctrl.showMenu);
    },

    showMenu: function() {

        var wrapper = model.elements.wrapper.children('.wrapper');

        wrapper.fadeOut('fast', function() {
            wrapper.remove();
        });

        model.elements.portfolio.animate({top: 0}, 500);
        model.elements.biography.animate({top: 0}, 500);
        model.elements.blog.animate({left: 0}, 500);
        model.elements.logo.fadeIn('slow');
        model.elements.contact.animate({bottom: 0}, 500);
        model.elements.snippets.animate({right: 0}, 500);
    }

}

ctrl.setElementBindings();
});

Does the hideMenu and showMenu fall under the view or controller?

UI only changes could definitely be put in the view.

  Have I put viewNewPage in the correct block?

I would put data changing code in the controller, so no.

  Should I bind elements to functions in the controller or model?

I would bind elements to events in the view.

  Have I made any other blunders here when trying to follow the MVC pattern?

The view is a combination of the view javascript and the corresponding html. The html selectors shouldn't exist outside of the view, and absolutely not in the model.

model.elements => view.elements

hideMenu, showMeny, setElementBindings => view

viewNewPage => controller


String Repeat function in VBA VBA has built-in functions for repeating a single character:

  Function String$(Number As Long, Character) As String

  Function Space$(Number As Long) As String

But neither are of any use when you need to repeat a string that has more than one character.

You could repeat a string "abcde" 5 times by doing something crafty like:

?Join(Split(String$(5,"."),"."),"abcde")

But that is neither intuitive nor performant.

In Excel, there is also WorksheetFunction.Rept, but it is painfully slow, and only available in Excel.

So I made a custom function that builds the string, while minimizing the concatenations. In fact, it doesn't use any concatenation, but instead uses a buffer and CopyMemory to fill the buffer. And rather than filling the buffer one instance at a time, the code fills the buffer using a lookback that reduces the number of buffer writes exponentially:

Given a string "abcde" that repeats 5 times:

Create a buffer of 25 spaces

"                         "

1st buffer write - assign the string to the first buffer position

"abcde                    "
 [NEW]

2nd buffer write - copy the existing populated buffer (5 characters) into the next buffer position

"abcdeabcde               "
      [NEW]

3rd buffer write - copy the existing populated buffer (10 characters) into the next buffer position

"abcdeabcdeabcdeabcde     "
           [  NEW   ]`

4th buffer write - copy the lesser of the existing populated buffer (20 characters) and the remaining buffer (5 characters) into the next buffer position.

"abcdeabcdeabcdeabcdeabcde"
                     [NEW]

StringRepeat

Private Declare Sub CopyMemory Lib "kernel32.dll" Alias "RtlMoveMemory" (ByVal Destination As Long, ByVal source As Long, ByVal Length As Long)

Public Function StringRepeat(number As Long, expression As String) As String

  Dim copyBufferLength As Long
  copyBufferLength = LenB(expression)

  'Create a buffer
  StringRepeat = Space$(number * Len(expression))

  Dim bufferLengthBytes As Long
  bufferLengthBytes = LenB(StringRepeat)

  Dim bufferPointer As Long
  bufferPointer = StrPtr(StringRepeat)

  'Copy the original expression to the start of the buffer
  CopyMemory bufferPointer, StrPtr(expression), copyBufferLength

  Do While copyBufferLength < bufferLengthBytes
    Dim remainingByteCount As Long
    'Check we're not going to overflow the buffer
    remainingByteCount = bufferLengthBytes - copyBufferLength
    If copyBufferLength > remainingByteCount Then
      CopyMemory bufferPointer + copyBufferLength, bufferPointer, remainingByteCount
    Else
      CopyMemory bufferPointer + copyBufferLength, bufferPointer, copyBufferLength
    End If
    copyBufferLength = copyBufferLength * 2
  Loop

End Function

The performance varies by the number of repeats, and the number of characters in the string to be repeated. I tried handling special cases like repeating a string 1 time (just return the string), and/or repeating a single character (return the result of String$ instead), but while that speeds up the special cases, it slows down all other cases.

I'm not checking whether the number input is positive, and I'm not checking that the string to repeat is at least 1 character long, as for now, I'm focusing on performance.

In some instances (small values of number, short expression lengths), avoiding the Exponential lookback approach is not as fast as a straight-up loop and copy:

RepeatString Simple

Function StringRepeatSimple(number As Long, expression As String) As String

  Dim expressionLengthBytes As Long
  expressionLengthBytes = LenB(expression)

  'Create a buffer
  StringRepeatSimple= Space$(number * Len(expression))

  Dim bufferPointer As Long
  bufferPointer = StrPtr(StringRepeatSimple)

  Dim expressionPointer As Long
  expressionPointer = StrPtr(expression)

  Dim copyCounter As Long
  For copyCounter = 0 To number - 1
    CopyMemory bufferPointer + copyCounter * expressionLengthBytes, expressionPointer, expressionLengthBytes
  Next copyCounter

End Function

FWIW, this might be a rare case where the Mid statement is apropos here. This doesn't require any API. This should yield similar performance characteristics since we only allocate the buffer once just as we do with the API version.

Public Function Replicate(RepeatString As String, NumOfTimes As Long)
    Dim s As String
    Dim c As Long
    Dim l As Long
    Dim i As Long

    l = Len(RepeatString)
    c = l * NumOfTimes
    s = Space$(c)

    For i = 1 To c Step l
        Mid(s, i, l) = RepeatString
    Next

    Replicate = s
End Function


Duplicated code in web service consumption I have a set of classes that abstract away calls to a set of web services. I have 6 classes in this particular group, 4 of which contain a simple function that, while small, is still duplicated. What happens is that if some exception or business rule violation happens in the service, they package it up as a fault object in the response. Otherwise, the fault object is null. As a result, the functions simply check to see if that object is not null and, if so, take the reason provided and throw an exception up the chain. So in one class, it might look like

private void ThrowIfContainsFault(AlphaResponse response)
{
    if (response.Fault != null)
    {
        throw new WhateverException(response.Fault.reasonText);
    }
}

And then the other classes, the response object will be of a different type, but the Fault property is the same and the block of code is the same.

private void ThrowIfContainsFault(BravoResponse response)
private void ThrowIfContainsFault(CharlieResponse response)
private void ThrowIfContainsFault(DeltaResponse response)

(Note: These class names are changed for readability, there is no common ancestor for these response objects.)

My first thought is that I could simply change the functions to receive the Fault object directly and forget the response object, and that's valid. But my overall concern is that while these methods literally do the same thing, the classes themselves are not particularly related, so introducing a common base hierarchy for a single (protected) method does not strike me as a suitable fit.

Here's my throwaway thought that I'm not even sure I like:

Define an empty interface that each class can "implement," then define an extension method against that interface.

internal interface IFaultThrower
{
}

internal static class IFaultThrowerExtension
{
    internal static void ThrowIfNotNull(this IFaultThrower thrower, Fault fault)
    {
        if (fault != null)
        {
            throw new WhateverException(fault.reasonText);
        }
    }
}

This would allow the related classes to get rid of their own private methods and invoke this common one.

 // this.ThrowIfContainsFault(response);
 this.ThrowIfNotNull(response.Fault);

Again, not sure I like it, but I'm also not sure I like the same general idea creeping into multiple classes, either.

I think it is a bad idea to introduce an empty interface to just add an extension method to it. You should think about an interface as a contract and not merely a way to introduce syntax sugar :)

I believe instead of playing with extension methods it is enough to introduce a separate, helper, internal class whose only purpose would be to throw an exception if fault object is null:

    internal class FaultResponseChecker
    {
        internal static void ThrowIfNotNull(Fault fault)
        {
            if (fault != null)
            {
                throw new WhateverException(fault.reasonText);
            }
        }
    }

If you, as I, don't like static methods, then you can use IoC container to inject  IFaultResponseChecker into the constructor of every class.


Beginner number guessing game I am a beginner to Java and have just written a number guessing game which involves guessing numbers from 1 to 10. I would like to improve my code by sticking strictly to conventions, improving code efficiency and forming good coding habits from the beginning.

Main class:

public class Main {

    public static void main(String[] args) {
        while (GuessingGame.play) {
            GuessingGame.play();
        }
    }
}

GuessingGame class:

import java.util.Random;
import java.util.Scanner;

public class GuessingGame {

    public static boolean play = true;

    private static int number = 0;
    private static int guess = 0;
    private static Random random = new Random();
    private static Scanner scanner = new Scanner(System.in);

    private static int nextInt(Random r, int lower, int higher) {
        int ran = r.nextInt(higher - lower);
        return (int) lower + ran;
    }

    public static void play() {
        number = nextInt(random, 1, 10);
        System.out.println("Insert your guess from 1-10!");
        guess = scanner.nextInt();
        if (guess == number) {
            System.out.println("Correct!");
            play = false;
        } else {
            System.out.println("Wrong! The number is: " + number);
            System.out.print("Do you want to try again? Y/N");
            String s = scanner.next();
            play= s.equalsIgnoreCase("Y") ? true : false;
        }
    }
}

There's no point in writing a separate class just to contain the main() function.  I'd rather invoke java GuessingGame than java Main.

while (GuessingGame.play) is bad in several ways:

GuessingGame exposes a public variable.  It's standard practice to require Main to call a method, so that no other class can write to a variable in the GuessingGame class.
The method GuessingGame.play() and the variable GuessingGame.play are too similarly named, which is confusing.
GuessingGame.play() sets the GuessingGame.play flag as a side-effect, which is unconventional and unintuitive.  It also violates the Single Responsibility Principle: play() should do one thing only.
A do-while loop might be more appropriate, since you want to play at least one game.

In GuessingGame, everything is static.  The number and guess variables could easily be made into local variables in play(), and therefore they should be.

Since the Random object is a class variable, there is no point in passing it as a parameter to nextInt().  There is a bug in nextInt(), in that it will never generate a 10.

Suggested solution

import java.util.Random;
import java.util.Scanner;

public class GuessingGame {

    private static Random random = new Random();
    private static Scanner scanner = new Scanner(System.in);

    /**
     * Picks a number uniformly between lowerBound (inclusive)
     * and upperBound (exclusive).
     */
    private static int nextInt(int lowerBound, int upperBound) {
        return lowerBound + random.nextInt(upperBound - lowerBound);
    }

    /**
     * Plays one game.
     *
     * @return true if the guess was correct
     */
    public static boolean play() {
        int number = nextInt(1, 10 + 1);
        System.out.println("Insert your guess from 1-10!");
        if (scanner.nextInt() == number) {
            System.out.println("Correct!");
            return true;
        } else {
            System.out.println("Wrong! The number is: " + number);
            return false;
        }
    }

    /**
     * Asks if the user wants to play again.
     */
    public static boolean shouldPlayAgain() {
        System.out.print("Do you want to try again? Y/N ");
        return "Y".equalsIgnoreCase(scanner.next());
    }

    /**
     * Plays guessing games until the user guesses correctly or gives up.
     */
    public static void main(String[] args) {
        while (!play() && shouldPlayAgain());
    }
}


Is the JavaScript code I've provided following Functional Programming best-practices? I'm trying to wrap my head around JavaScript Functional Programming.

I have created a very basic script that creates a new elements and appends them to the DOM.

If anyone could share any feedback, or great resources on the subject, that would be awesome.

Where does this style of programming lend itself best? Web Development, or more Software development? I understand that React uses this coding paradigm. I'm actually very comfortable using React - but, I'm wondering how it might be best utilised with vanilla JavaScript

Thanks!

// create a button that accepts text and a callback function

const createButton = (text, func) => {
    const button = document.createElement('button');
    const buttonText = document.createTextNode(text);
    button.appendChild(buttonText);

    button.addEventListener('click', () => {
        return func();
    });

    return button;
};

// create paragraph tag that accepts a text arg

const createText = (text) => {
    const pTag = document.createElement('p');
    const pTagText = document.createTextNode(text);
    pTag.appendChild(pTagText);

    return pTag;
};

// pure functions

const myList = [1, 2, 3];

const sayHello = (name) => {
    return `Hello, ${name}!`;
};

const addNums = (x, y) => {
    return x + y;
};

const simpleLog = () => {
    return 'This is a simple log';
};

const addToArray = (item) => {
    return [...myList, item];
}

// view - component that will get rendered to the DOM

const view = () => {
    const component = document.createElement('div');

    // create a button element that has a callback function that will append the returned value to the DOM

    component.appendChild(createButton('Hello', () => {
        return component.appendChild( createText( sayHello('Kristian') ) );
    }));

    component.appendChild(createButton('Add Numbers', () => {
        return component.appendChild( createText( addNums(3, 7) ) );
    }));

    component.appendChild( createButton('Log', simpleLog) );

    component.appendChild(createButton('Add to array', () => {
        console.log( `Original array: [${myList}] - New array: [${addToArray(4)}]`);
    }));

    return component;
};

// grab the el that you want to append the view to

const rootNode = document.getElementById('app');

// append view component to el

rootNode.appendChild(view());

Preface
"Best Practises" are very subjective, so your question cannot really be answered in that regard, but I will try to give you some tips nonetheless.
To lead into the rest of my answer I would say one of the most beginner-friendly resources for learning a functional style in javascript is this Medium article (without going into a lot of the nitty-gritty theoretical stuff). By reading this I think you will come to understand a lot of the ideas that come with functional programming.

♀ Paradigm vs. Style
With that out of the way, let us start with a shocker: Javascript is not really a functional programming language. Yep, I said it. Javascript is more a procedural (or object-oriented) language (some newer features have allowed for more functional-ish style but it remains very procedural). But all that ofcourse does not mean you cannot apply a functional style, which I think is more the core of your question?
Presuming I did correctly interpret your question, let me paraphrase to you the first two sentences of the Wikipedia page about FP:

[...] functional programming is a programming paradigm where programs are constructed by applying and composing functions. It is a declarative programming paradigm [...], rather than a sequence of imperative statements [...].

This means that, when writing your programs, you do not tell the computer exactly what to do every step of the way (i.e. for (let i = 0; i < n; i++)) but rather you chunk a problem into smaller sub-problems, solve those, and compose a function that calls your smaller functions to solve the bigger problem. This leads me to my first tip. Javascript functions come in two forms: procedures, and lambda's. I am not going into the details that differentiate the two, but rather I would like to tell you about lambda's.

 Tip 1: Embrace expressions
You seem to know javascript's lambda syntax, since you use the arrow notation (x => y), but you are using it in a procedural way. What I mean by that, is that you append a block to each lambda, as such:
const doSomething = (arg1, arg2) => { /*steps*/ };

This kind of defeats the purpose of the notation, and is basically another syntax for the procedural functions of JS:
function doSomething (arg1, arg2) { /*steps*/ }

In Javascript, x => y means a function that takes x and returns y. So a lot of your functions can be simplified. Let's change a couple to make it clear:
const sayHello = (name) => {
    return `Hello, ${name}!`;
};
// can be
const sayHello = name => `Hello ${name}!`;

and
const addNums = (x, y) => {
    return x + y;
};
// can be
const add = (x, y) => x + y;

 Tip 2: Spice up your functions
But then what is the purpose of the lambda notation, and why is your code defeating its purpose? To put it simply: a lambda is a function that takes zero or one argument(s), maybe transforms it, and yields a result. Wikipedia's article about lambda calculus says the same, but with more jargon:

[...] all functions in the lambda calculus are anonymous functions, having no names. They only accept one input variable, with currying used to implement functions with several variables.

You use a lot of functions as procedures (sequence of steps) which you cannot have in FP, and you are using multiple parameter functions which are also disallowed (or rather just do not exist in FP). But, you might ask, how are you supposed to do anything with just one parameter and a single expression body? Let us take a snippet of your code and refactor it into a nice functional style:
// Original
const createButton = (text, func) => {
    const button = document.createElement('button');
    const buttonText = document.createTextNode(text);
    button.appendChild(buttonText);

    button.addEventListener('click', () => {
        return func();
    });

    return button;
};

// Functional alternative
// Some setup, notice the currying for multiple arguments.
const text = text => document.createTextNode(text);
const newButton = () => document.createElement("button");
const listen = event => handler => target => {
  // Javascript was not meant to be used purely functionally,
  // so we have to use a block with several statements to
  // mutate the target, and then return it (mutation will never
  // happen in functional programming, as you might know).
  target.addEventListener(event, handler);
  return target;
};
const append = child => target => {
  // Same story: we have to return the target after mutating it.
  target.append(child);
  return target;
};

// Here comes the true power of functional programming: composition
const onClick = listen("click");
const addLabel = label => append(text(label));
const sayHi = () => console.log("Hi!");
const sayBye = () => console.log("Bye.");
const button = label => handler =>
  addLabel(label)(
    onClick(handler)(
      newButton()));

const friendlyButton = button(":)")(sayHi);
const sadButton = button(":(")(sayBye);

// Try it out!
document.body.append(friendlyButton, sadButton);

Ok, giant code sample with weird and maybe hideous things. Let me explain it!
Firstly I would like to say that Javascript is a great language, but the DOM is not the best part to use FP for, because it's an amalgamation of mutation and imperativeness, which are the arch-enemies of FP. But let us do it anyways because it is fun!
The first statement of the refactor goes as follows:
const text = text => document.createTextNode(text);

which is essentially a function that, when called with an argument text will create a textNode with text being its text. Now this is a bit of a weird one, because we made a function that takes an argument, and then calls another function with that very argument, so we basically aliased it. Aliasing is the same as storing it in a variable with another name, so why not write it like this:
const text = document.createTextNode;

text is equal to document.createTextNode, so text("Hi!") is equal to document.createTextNode("Hi!").

Now JS is a bit weird (and full of side-effects), so we have to specify that the this of document.createElement is document (document.createElement.bind(document)). The peculiarities of Javascript's this go far beyond the purpose of this post so if you do not understand what that meant, just wrap it in a function like so: x => y(x);

The first real weirdness happens in the listen function:
const listen = event => handler => target => { /*...*/ };

What? A function that returns a function that returns a function that returns something else?! Yep, that is functional programming baby! But why would we do this? Because it allows for very easy composition. Notice how later on, we defined the function onClick, which is a composed listen:
const onClick = listen("click");
// This means the following:
const onClick = handler => target => target.addEventListener("click", handler);

So we made a function that always adds a click eventlistener to a target. We can take this a step further:
// Let's make a function that makes an element yell "OUCH!" on click!
const ouch = onClick(() => console.log("OUCH!"));
// This means the following:
const ouch = target => target.addEventListener("click", () => console.log("OUCH!"));

// Try it out!
const boxingDummy = ouch(addLabel("Don't hit me!")(newButton()));
document.body.append(boxingDummy);

I hope that made it clear why you would write programs in such a weird (coming from procedural programming) way. I would link you the wiki article but that is just unintelligble jibberish to me too.

 Tip 3: Lose control
You seem to really rely on the imperativeness of Javascript to perform tasks in a set order, which does not fit well in a functional style -- most functional languages do not even allow it. The great functional programming evangelists have thought about this long and hard, and showed us how you do not need loops, if's or sequences to create complete and functional (in the literal sense) programs. Taking, for example, another function you wrote:
const view = () => {
    const component = document.createElement('div');

    // create a button element that has a callback function that will append the returned value to the DOM

    component.appendChild(createButton('Hello', () => {
        return component.appendChild( createText( sayHello('Kristian') ) );
    }));

    component.appendChild(createButton('Add Numbers', () => {
        return component.appendChild( createText( addNums(3, 7) ) );
    }));

    component.appendChild( createButton('Log', simpleLog) );

    component.appendChild(createButton('Add to array', () => {
        console.log( `Original array: [${myList}] - New array: [${addToArray(4)}]`);
    }));

    return component;
};

Pardon the joke, but this looks really WET... (as in, the opposite of DRY). You call component.appendChild(createButton(/*...*/)) a lot of times. Apart from that being a bit of a code smell, it also is a really great place to put in some FP magic. Analysing the abstract meaning of your code, let me write that out in a more functional style:
// Remember our previous functions?
// I made them nice and generic so they are
// reusable here!
const appendButton = label => action => append(button(label)(action));
const add = x => y => x + y;
const addFive = add(5);
const saySix = () => console.log(addFive(1));
const fold = (...functions) => value => functions.reduce(
  (result, fun) => fun(result),
  value);
const component = document.createElement("div");
const view = fold(
  appendButton("Greet")(sayHi),
  appendButton("What's five plus one?")(saySix)
  /* etc... */);

// Try it out!
document.body.append(view(component));

BAM! No blocks, no loops, no if's! Instead of a sequence of statements, we use a fold to abstract away a lot of the DOM boilerplate. Higher-order functions such as fold(left) are very important functions to keep in your arsenal. I suggest reading up on it a bit, if the article isn't too crytographic (there's a joke that says functional programmers tend to lose their ability to explain FP as soon as they understand, and I think that applies perfectly on most wiki pages).

Well that should be about it. To summarise:

Javascript DOM isn't the best place to apply FP (in my humble opinion), but go ahead and break the rules!
Don't get RSI: (x) => { return y; } is the same as x => y.
Procedures are so 1970; compose a function from functions to solve your problems (and most importantly: be able to reason about your solution).
Currying and higher-order functions are a functional programmer's best friends, conquer the world with them.


Close to the metal C++ Palindrome Program I am attempting to reinvent the wheel by writing my own palindrome checking program in C++, using only basic loops and mathematical operations. I have arrived at a solution, but was wondering if it's the best solution and if it could be improved. I'm doing this primarily as a learning exercise. My code:

#include <iostream> // cout
#include <stdio.h>  // printf
#include <string>

using namespace std;

bool is_palidrome(string inputString) {
  int num_to_parse = inputString.size();

  /* immediately return on single letter string as all single letter strings are palindromes -- saves CPU time as programme does not need to enter the loop */
  if(num_to_parse == 1) {
      return true;
  }

  for(int i=0; num_to_parse; i++) {
      int point_end = num_to_parse - i - 1;
    if(inputString[i] == inputString[point_end]) {
      continue;
    } else if (i == num_to_parse) {
      return true;
    } else {
      return false;
    }
}}

int main() {
    // Change the value of inputString to test different palindromes
    string inputString = "caabaac";
    printf("%s", is_palidrome(inputString)?"This is a palindrome.":"Not a palindrome."); // ternary: test condition ? return this if true : return this if false
    return 0;
}

I think at the moment I'm doing about 2x the number of loops I need to do, and so wasting CPU time. Also, there may be further optimizations. Any help really appreciated as I really want to understand this stuff properly and contribute to code-bases, rather than just doing everything with other people's higher-level code.

for(int i=0; num_to_parse; i++)

wut

Why is num_to_parse your condition? Then you have if (i == num_to_parse) in there, too? That's odd. I'd rewrite this as:

for(int i=0; i < num_to_parse; i++) {
  int point_end = num_to_parse - i - 1;
  if(inputString[i] == inputString[point_end]) {
    continue;
  } else {
    return false;
  }
}
return true;

If you want to simplify that even further, you can just do this:

for(int i=0; i <= num_to_parse; i++) {
  int point_end = num_to_parse - i - 1;
  if(inputString[i] != inputString[point_end]) {

    return false;
  }
}
return true;

Take note of the different condition. There are a couple of other simplifying improvements, but they're mentioned in another answer.

While I was writing that, I realized that your indentation is... odd, to say the least. Please don't use things like }} in code; while there are a few rare cases where it might be OK (nested namespaces that, as is typical, you didn't indent), none of them apply here.

using namespace std is a bad idea. Please don't do it.

printf("%s", is_palidrome(inputString)?"This is a palindrome.":"Not a palindrome.");

This is just... weird. I mean, first of all, you don't have a trailing newline, so it might never output anything at all. In my C IDE, for example, it would, but on my Bash console, it'd definitely look odd (This is a palindrome.q@my-hostname ~>), if it prints at all. Even ignoring that inconsistency... why do it like that? The ternary just makes it more complicated than it has to be.

If you insist on using C functions (more on that later), do this:

if (is_palindrome(inputString)) {
  puts("This is a palindrome.");
} else {
  puts("Not a palindrome.");
}

The C++ equivalent should be fairly straightforward; replace the puts(...) with std::cout << ... << "\n";, or std::endl in place of "\n" if you want to be paranoid about flushing every single line instead of just ensuring that it's flushed in a few key places (which is what I do in performance-critical stuff, because writing to the console is slow, and until you tell them to, most C++ standard libraries will avoid doing it for as long as is reasonable)

Now to talk about that "using C functions" thing. Why are you doing it at all? You have all of C++'s power to bring to bear, but you don't. If you want to write this in C, write it in C -- it'd only be changing, like four lines. Maybe less. Don't use just a tiny part of C++. Or, alternatively, just write C.

With regards to your original concern about speed, you're doing this in O(n) (as far as I can tell), which is the absolute lower limit. You're still iterating over every character, though, so it's not as fast as it could get -- you only need to iterate over half.

Finally, what do you mean by "close to the metal"? This is just C with a tiny sprinkling of C++, which are both high-level languages. Not as high-level as, say, Python, but the definition of a high-level language is that it's abstracted from any one particular machine, which C and C++ are. There's nothing close to the metal here. That would be machine code, or maybe Assembly.


Is this an acceptable implementation of the Adapter pattern? I've used the UK Plug -> US Socket and US Plug -> UK Socket analogy. I've also made it possible for UK plug to use a UK socket and a US plug to use a US socket. The output of the SSCCE below is

UKPlug -> UKSocket
USPlug -> USSocket
UKPlug -> UKUSAdapter -> USSocket
USPlug -> USUKAdapter -> UKSocket

And the code

package com.structural.adapter.adapter2;

interface IAdapter {
  public void plugin();
}

class UKUSAdapter implements IAdapter {
  UKPlug plug;
  USSocket socket;
  public UKUSAdapter(UKPlug plug, USSocket socket) {
    this.plug = plug;
    this.socket = socket;
  }

  @Override
  public void plugin() {
    System.out.print("UKUSAdapter -> ");
    socket.accept_2pin_plug(plug.pin1, plug.pin2);
  }
}

class USUKAdapter implements IAdapter {
  USPlug plug;
  UKSocket socket;
  public USUKAdapter(USPlug plug, UKSocket socket) {
    this.plug = plug;
    this.socket = socket;
  }

  @Override
  public void plugin() {
    System.out.print("USUKAdapter -> ");
    socket.accept_3pin_plug(plug.pin1, plug.pin2, 1);
  }

}

class USSocket {
  USPlug plug;
  public USSocket(USPlug plug) {
    this.plug = plug;
  }

  public void accept_2pin_plug(int pin1, int pin2) {
    System.out.print("USSocket");
  }
}

class UKSocket {
  UKPlug plug;
  public UKSocket(UKPlug plug) {
    this.plug = plug;
  }
  public void accept_3pin_plug(int pin1, int pin2, int pin3) {
    System.out.print("UKSocket");
  }
}

class UKPlug {
  int pin1;
  int pin2;
  int pin3;

  IAdapter adapter;
  UKSocket socket;

  public UKPlug(int pin1, int pin2, int pin3) {
    this.pin1 = pin1;
    this.pin2 = pin2;
    this.pin3 = pin3;
  }

  public void plug_into_3pin() {
    System.out.print("UKPlug -> ");
    if(this.socket == null) {
      adapter.plugin();
    } else {
     socket.accept_3pin_plug(pin1, pin2, pin3);
    }
  }

  public void setAdapter(IAdapter adapter) {
    this.adapter = adapter;
    this.socket = null;
  }

  public void setSocket(UKSocket socket) {
    this.socket = socket;
    this.adapter = null;
  }
}

class USPlug {
  int pin1;
  int pin2;

  IAdapter adapter;
  USSocket socket;

  public USPlug(int pin1, int pin2) {
    this.pin1 = pin1;
    this.pin2 = pin2;
  }

  public void plug_into_2pin() {
    System.out.print("USPlug -> ");
    if(this.socket == null) {
      adapter.plugin();
    } else {
      socket.accept_2pin_plug(pin1, pin2);
    }
  }

  public void setAdapter(IAdapter adapter) {
    this.adapter = adapter;
    this.socket = null;
  }

  public void setSocket(USSocket socket) {
    this.socket = socket;
    this.adapter = null;
  }
}

public class TestPlugAdapters {
  public static void main(String[] args) {
    UKPlug ukPlug = new UKPlug(1,1,1);
    UKSocket ukSocket = new UKSocket(ukPlug);

    USPlug usPlug = new USPlug(1,1);
    USSocket usSocket = new USSocket(usPlug);

    //Test UK Plug to UK Socket
    ukPlug.setSocket(ukSocket);
    ukPlug.plug_into_3pin();
    System.out.println();

    //Test US Plug to US Socket
    usPlug.setSocket(usSocket);
    usPlug.plug_into_2pin();
    System.out.println();

    //Test UK Plug to US Socket
    IAdapter ukusAdapter = new UKUSAdapter(ukPlug, usSocket);
    ukPlug.setAdapter(ukusAdapter);
    ukPlug.plug_into_3pin();
    System.out.println();

    //Test US Plug to UK Socket
    IAdapter usukAdapter = new USUKAdapter(usPlug, ukSocket);
    usPlug.setAdapter(usukAdapter);
    usPlug.plug_into_2pin();
    System.out.println();
  }
}

This is not an implementation of the Adapter pattern.

What is the Adapter pattern trying to solve?

Lets' say I have :

public interface UKPlug {

    int getPin1();

    int getPin2();

    int getPin3();
}

public class UKSocketImpl {

    public void plugin(UKPlug plug) {

        if (plug.getPin1() == -plug.getPin2() && plug.getPin3() == 0) {
            System.out.println(plug.toString() + " -> UKSocket");
        }

    }
}

A UK socket, that accepts UKPlugs. The socket is the client in this scenario, which makes use of an interface (UKPlug)

Now I want to plug in my device, but it has a USPlug :

import java.math.BigInteger;

public class USPlugImpl {

    public BigInteger getPin1() {
        return BigInteger.TEN;
    }

    public BigInteger getPin2() {
        return BigInteger.TEN.negate();
    }

    @Override
    public String toString() {
        return "USPlug";
    }
}

This is where we can write an Adapter class to adapt our Adaptee (USPlugImpl) :

package adapter.improved;

public class USUKAdapter implements UKPlug {

    private final USPlugImpl plug;

    public USUKAdapter(USPlugImpl plug) {
        this.plug = plug;
    }

    @Override
    public int getPin1() {
        return plug.getPin1().intValue();
    }

    @Override
    public int getPin2() {
        return plug.getPin2().intValue();
    }

    @Override
    public int getPin3() {
        return 0;
    }

    @Override
    public String toString() {
        return plug.toString() + " adapted to UKPlug";
    }
}

This is what the Adapter class does :

it implements the needed interface
it does so by delegating to methods of the Adaptee instance (the USPlug), or by supplying some default behaviour.
Both the USPlug as the UKSocket are unaware of the use of an Adapter.

example use :

private void plugIn(UKSocketImpl ukSocket, USPlugImpl usPlug) {
    ukSocket.plugin(new USUKAdapter(usPlug));
}

How do your Adapter classes differ?

They implement an interface, but not one that is used by the Client
The UKPlug and USPlug classes have a reference to an adapter. What if these were classes from a 3rd party library, and you couldn't add members to them? That's what the Adapter pattern solves.

Final style note : don't use '_' in method names.


Track the highest high in a time series I want to find the highest number in a time series, in the most efficient way.
To avoid iterating through all the look back periods, I'm only iterating in case the current high number has just been discarded. I'm avoiding enqueue dequeue for the same performance reasons.

What would be a faster algorithm?

class Program
{
    static Random r = new Random();
    static int LookBackPeriods = 5; //(small to facilitate visual functional test). eventually could be 100 1000, but not more than 5000.
    static double High=0;
    static int HighLocation = 0;
    static int CurrentLocation = 0;
    static double[] arr = new double[LookBackPeriods];

    static void Main(string[] args)
    {
        //functional test - generate 100 numbers and check them in as you go:
        for (int i = 0; i < 100; i++)
            CheckInCandidate(r.Next(10)); //in final application these will be real numbers
        Console.ReadLine();
    }

    /// <summary>
    /// A full iteration on all numbers (run only when the current high is overwritten)
    /// </summary>
    static void ReFindHighest()
    {
        High = arr[0];
        HighLocation =0
        for (int i = 1; i < LookBackPeriods; i++)
            if (arr[i] > High)
            {
                High = arr[i];
                HighLocation = i;
            }
    }
    /// <summary>
    /// minimized check that should run most of the time a new number arrives
    /// </summary>
    static void CheckInCandidate(double currentNum)
    {
        arr[CurrentLocation] = currentNum;
        if (currentNum >= High)
        {
            HighLocation = CurrentLocation;
            High = currentNum;
        }
        else if (CurrentLocation == HighLocation)
            ReFindHighest();

        CurrentLocation++;
        //if (CurrentLocation == HistoryLength) CurrentLocation=0; //is this faster??
        CurrentLocation = CurrentLocation % LookBackPeriods; //is this faster??
        Console.WriteLine("Current:" + currentNum + "\tHighest of " + LookBackPeriods + "last is " + High); //for validation purposes only.
    }
}

It seems that the above caching works much faster than the Ascending Minima algorithm.
the code below contains both algos with benchmarks (this version is for finding the lowest).

static class Program
{
    static Random r = new Random();
    static int Window = 1000; //(small to facilitate visual functional test). eventually could be 100 1000, but not more than 5000.
    const int FullDataSize =100000;//000;
    static double[] InputArr = new double[FullDataSize]; //array prefilled with the random input data.

    //====================== Caching algo variables
    static double Low = 0;
    static int LowLocation = 0;
    static int CurrentLocation = 0;
    static double[] Result1 = new double[FullDataSize]; //contains the caching mimimum result
    static int i1; //incrementor, just to store the result back to the array. In real life, the result is not even stored back to array.

    //====================== Ascending Minima algo variables
    static double[] Result2 = new double[FullDataSize]; //contains ascending miminum result.
    static double[] RollWinArray = new double[Window]; //array for the caching algo
    static Deque<MinimaValue> RollWinDeque = new Deque<MinimaValue>(); //Niro.Deque nuget.
    static int i2; //used by the struct of the Deque (not just for result storage)

    //====================================== my initialy proposed caching algo
    static void CalcCachingMin(double currentNum)
    {
        RollWinArray[CurrentLocation] = currentNum;
        if (currentNum <= Low)
        {
            LowLocation = CurrentLocation;
            Low = currentNum;
        }
        else if (CurrentLocation == LowLocation)
            ReFindHighest();

        CurrentLocation++;
        if (CurrentLocation == Window) CurrentLocation = 0; //this is faster
        //CurrentLocation = CurrentLocation % Window; //this is slower, still over 10 fold faster than ascending minima

        Result1[i1++] = Low;
    }

    //full iteration run each time lowest is overwritten.
    static void ReFindHighest()
    {
        Low = RollWinArray[0];
        LowLocation = 0; //bug fix. missing from initial version.
        for (int i = 1; i < Window; i++)
            if (RollWinArray[i] < Low)
            {
                Low = RollWinArray[i];
                LowLocation = i;
            }
    }

    //======================================= Ascending Minima algo based on http://stackoverflow.com/a/14823809/2381899
    private struct MinimaValue
    {
        public int RemoveIndex { get; set; }
        public double Value { get; set; }
    }

    public static void CalcAscendingMinima (double newNum)
    { //same algo as the extension method below, but used on external arrays, and fed with 1 data point at a time like in the projected real time app.
            while (RollWinDeque.Count > 0 && i2 >= RollWinDeque[0].RemoveIndex)
                RollWinDeque.RemoveFromFront();
            while (RollWinDeque.Count > 0 && RollWinDeque[RollWinDeque.Count - 1].Value >= newNum)
                RollWinDeque.RemoveFromBack();
            RollWinDeque.AddToBack(new MinimaValue { RemoveIndex = i2 + Window, Value = newNum });
            Result2[i2++] = RollWinDeque[0].Value;
    }

    public static double[] GetMin(this double[] input, int window)
    {   //this is the initial method extesion for ascending mimima
        //taken from http://stackoverflow.com/a/14823809/2381899
        var queue = new Deque<MinimaValue>();
        var result = new double[input.Length];

        for (int i = 0; i < input.Length; i++)
        {
            var val = input[i];

            // Note: in Nito.Deque, queue[0] is the front
            while (queue.Count > 0 && i >= queue[0].RemoveIndex)
                queue.RemoveFromFront();

            while (queue.Count > 0 && queue[queue.Count - 1].Value >= val)
                queue.RemoveFromBack();

            queue.AddToBack(new MinimaValue { RemoveIndex = i + window, Value = val });

            result[i] = queue[0].Value;
        }

        return result;
    }

    //============================================ Test program.
    static void Main(string[] args)
    { //this it the test program.
        //it runs several attempts of both algos on the same data.
        for (int j = 0; j < 10; j++)
        {
            Low = 12000;
            for (int i = 0; i < Window; i++)
                RollWinArray[i] = 10000000;
            //Fill the data + functional test - generate 100 numbers and check them in as you go:
            InputArr[0] = 12000;
            for (int i = 1; i < FullDataSize; i++) //fill the Input array with random data.
                //InputArr[i] = r.Next(100) + 11000;//simple data.
                InputArr[i] = InputArr[i - 1] + r.NextDouble() - 0.5; //brownian motion data.

            Stopwatch stopwatch = new Stopwatch();
            stopwatch.Start();
            for (int i = 0; i < FullDataSize; i++) //run the Caching algo.
                CalcCachingMin(InputArr[i]);

            stopwatch.Stop();
            Console.WriteLine("Caching  : " + stopwatch.ElapsedTicks + " mS: " + stopwatch.ElapsedMilliseconds);
            stopwatch.Reset();

            stopwatch.Start();
            for (int i = 0; i < FullDataSize; i++) //run the Ascending Minima algo
                CalcAscendingMinima(InputArr[i]);

            stopwatch.Stop();
            Console.WriteLine("AscMimima: " + stopwatch.ElapsedTicks + " mS: " + stopwatch.ElapsedMilliseconds);
            stopwatch.Reset();

            i1 = 0; i2 = 0; RollWinDeque.Clear();
        }

        for (int i = 0; i < FullDataSize; i++) //test the results.
            if (Result2[i] != Result1[i]) //this is a test that algos are valid. Errors (mismatches) are printed.
                Console.WriteLine("Current:" + InputArr[i].ToString("#.00") + "\tLowest of " + Window + "last is " + Result1[i].ToString("#.00") + " " + Result2[i].ToString("#.00") + "\t" + (Result1[i] == Result2[i])); //for validation purposes only.

        Console.ReadLine();
    }

}


Calculation of an inflation on volume/year I would like to find out if my current solution to the problem described below is "good enough" or if there is an alternative way of achieving it. All I care about is the length (no. of lines) of the code and its efficiency. I am trying to follow the "DRY" principle and come up with something that when seen by another developer in the future will be considered a "good practice".
I am forced[1] to enter a few formulas into spreadsheet cells via vba. The formula is for a calculation of an inflation (say labour rate) on volume/year. It has some constants and some variants but the tricky bit is a variable inflation rate added each year. In my real life example this is far more complicated but I have shortened it and came up with an SSCCE.
SSCCE
You can view the example as a spreadsheet on Google Docs, or have a quick look through below:

The formulas are:
D6 = -$B$12*D4*(D8+1)
E6 = -$B$12*E4*(D8+1)*(E8+1)
F6 = -$B$12*F4*(D8+1)*(E8+1)*(F8+1)
G6 = -$B$12*G4*(D8+1)*(E8+1)*(F8+1)*(G8+1)
H6 = -$B$12*H4*(D8+1)*(E8+1)*(F8+1)*(G8+1)*(H8+1)
I6 = -$B$12*I4*(D8+1)*(E8+1)*(F8+1)*(G8+1)*(H8+1)*(I8+1)
J6 = -$B$12*J4*(D8+1)*(E8+1)*(F8+1)*(G8+1)*(H8+1)*(I8+1)*(J8+1)
K6 = -$B$12*K4*(D8+1)*(E8+1)*(F8+1)*(G8+1)*(H8+1)*(I8+1)*(J8+1)*(K8+1)
L6 = -$B$12*L4*(D8+1)*(E8+1)*(F8+1)*(G8+1)*(H8+1)*(I8+1)*(J8+1)*(K8+1)*(L8+1)

I tried it but didn't like it. There are 9 really long and ugly lines (remember, in my real life example there are many more variables).
Range("D6").Formula = "-$B$12*D4*(D8+1)"
Range("E6").Formula = "-$B$12*E4*(D8+1)*(E8+1)"
Range("F6").Formula = "-$B$12*F4*(D8+1)*(E8+1)*(F8+1)"
Range("G6").Formula = "-$B$12*G4*(D8+1)*(E8+1)*(F8+1)*(G8+1)"
Range("H6").Formula = "-$B$12*H4*(D8+1)*(E8+1)*(F8+1)*(G8+1)*(H8+1)"
Range("I6").Formula = "-$B$12*I4*(D8+1)*(E8+1)*(F8+1)*(G8+1)*(H8+1)*(I8+1)"
Range("J6").Formula = "-$B$12*J4*(D8+1)*(E8+1)*(F8+1)*(G8+1)*(H8+1)*(I8+1)*(J8+1)"
Range("K6").Formula = "-$B$12*K4*(D8+1)*(E8+1)*(F8+1)*(G8+1)*(H8+1)*(I8+1)*(J8+1)*(K8+1)"
Range("L6").Formula = "-$B$12*L4*(D8+1)*(E8+1)*(F8+1)*(G8+1)*(H8+1)*(I8+1)*(J8+1)*(K8+1)*(L8+1)"

Or Cells:
Cells(6, 4).Formula = "-$B$12*D4*(D8+1)"
Cells(6, 5).Formula = "-$B$12*E4*(D8+1)*(E8+1)"
Cells(6, 6).Formula = "-$B$12*F4*(D8+1)*(E8+1)*(F8+1)"
Cells(6, 7).Formula = "-$B$12*G4*(D8+1)*(E8+1)*(F8+1)*(G8+1)"
Cells(6, 8).Formula = "-$B$12*H4*(D8+1)*(E8+1)*(F8+1)*(G8+1)*(H8+1)"
Cells(6, 9).Formula = "-$B$12*I4*(D8+1)*(E8+1)*(F8+1)*(G8+1)*(H8+1)*(I8+1)"
Cells(6, 10).Formula = "-$B$12*J4*(D8+1)*(E8+1)*(F8+1)*(G8+1)*(H8+1)*(I8+1)*(J8+1)"
Cells(6, 11).Formula = "-$B$12*K4*(D8+1)*(E8+1)*(F8+1)*(G8+1)*(H8+1)*(I8+1)*(J8+1)*(K8+1)"
Cells(6, 12).Formula = "-$B$12*L4*(D8+1)*(E8+1)*(F8+1)*(G8+1)*(H8+1)*(I8+1)*(J8+1)*(K8+1)*(L8+1)"

But try to play with the right hand side of the statement. I see so much potential** in terms of optimising it, but the best I could have come up with in vba is:
Sub Main()

    Dim i As Long
    Dim s As String

    For i = 4 To 12
        s = s + "*(" + Cells(8, i).Address + "+1)"
        Cells(6, i).Formula = "=-$B$12*" + Cells(4, i).Address + s
    Next i

End Sub

I realize that I am concatenating Strings to achieve my goal. The code is working fine in this SSCCE and in my real life problem. It's not slow but I am purely curious if there is any other approach or if my current one can be optimized any further.
Questions

Can this be optimized any further?
Would you do this differently? If so, how? Why would your solution be any better than my current one?

[1] Forced because the real life example pulls its values from multiple places like another closed workbook, database, website, user input etc. For example, INDIRECT() does not work with closed workbooks so I can't rely on pure built-in functions.

Here is what I would do if I were you:

Sub Main()
    Dim b12_value As Double
    Dim inflation As Double

    'Seed the inflation variable and store the static B12 value
    inflation = 1
    b12_value = Range("$B$12").value

    'Loop through each cell in D6:L6 and calculate its value.
    For Each cell In Range("D6:L6")
        inflation = inflation * (Cells(8, cell.Column).Value + 1)
        cell.Value = -b12_value * Cells(4, cell.Column).Value * inflation
    Next

End Sub

Since you are concerned about number of lines, I chose not to store the volume for each calulation in its own variable (which I would prefer to do as it makes for more readable code).

The solution is only 1 line longer than your original solution and it does not rely on string concatenation (which is slow). It also simply does the formula work itself instead of setting the cell's value instead of setting the formula then having Excel calculate the value from said formula.

NOTE: If you wanted the formula visible in Excel (instead of just its value), I would think your original solution (with a couple tweaks) is sufficient for you needs:

Sub Main()
    'Use a more descriptive variable name than "s". Descriptive names improve
    'readability and better facilitates user understanding.
    Dim inflation_string as String

    'Using the for each syntax creates more readable code. Plus, as a bonus,
    'it removed the use of the index variable i.
    For Each cell In Range("D6:L6")
        inflation_string = inflation_string + "*(" + Cells(8, cell.Column).Address + "+1)"
        cell.Formula = "=-$B$12*" + Cells(4, cell.Column).Address + inflation_string
    Next

End Sub


Priority Queue (Binary Search Tree) Implementation I have made a Priority Queue implemented as a Binary Search Tree. I would appreciate comments related to optimization and proper functionality, but any comments/critiques are welcome.

PQ.h:

#ifndef PQ_H
#define PQ_H

#include <iostream>

struct Node
{
    int value;
    Node * left;
    Node * right;
    Node(int value)
    : value(value), left(NULL), right(NULL)
    {

    }
};

class PQ
{
    private:
        Node * root;
        void Push(int value, Node * node);
        int Pop(Node * node, Node * parent);
        void DeletePQ(Node * node);
    public:
        PQ();
        ~PQ();
        void Push(int value);
        int Pop();
        void DeletePQ();
        bool IsEmpty();
};

#endif

PQ.C:

#include "PQ.h"

PQ::PQ()
{
    root = NULL;
}

PQ::~PQ()
{
    DeletePQ();
}

void PQ::DeletePQ()
{
    DeletePQ(root);
}

void PQ::DeletePQ(Node * node)
{
    if(node != NULL)
    {
        DeletePQ(node->left);
        DeletePQ(node->right);
        delete node;
    }
}

void PQ::Push(int value)
{
    if(root != NULL)
    {
        Push(value, root);
    }
    else
    {
        root = new Node(value);
    }
}

void PQ::Push(int value, Node * node)
{
    if(value < node->value)
    {
        if(node->left != NULL)
        {
            Push(value, node->left);
        }
        else
        {
            node->left = new Node(value);
        }
    }
    else
    {
        if(node->right != NULL)
        {
            Push(value, node->right);
        }
        else
        {
            node->right = new Node(value);
        }
    }
}

int PQ::Pop()
{
    int value;
    if(root != NULL)
    {
        if(root->right != NULL)
        {
            value = Pop(root->right, root);
        }
        else
        {
            value = root->value;
            if(root->left != NULL)
            {
                Node * temp = root;
                root = root->left;
                delete temp;
            }
            else
            {
                delete root;
                root = NULL;
            }
        }
    }
    else
    {
        value = -1;
    }
    return value;
}

int PQ::Pop(Node * node, Node * parent)
{
    int value;
    if(node->right != NULL)
    {
        value = Pop(node->right, node);
    }
    else
    {
        value = node->value;
        if(node->left != NULL)
        {
            parent->right = node->left;
        }
        else
        {
            parent->right = NULL;
        }
        delete node;
    }
    return value;
}

bool PQ::IsEmpty()
{
    bool isEmpty;
    if(root == NULL)
    {
        isEmpty = true;
    }
    else
    {
        isEmpty = false;
    }
    return isEmpty;
}

Handling booleans

Computation of isEmpty is anti-idiomatic. Consider

    isEmpty = (root == NULL);

Now it is easy to see that isEmpty is not needed at all. The whole method can be shortened to

bool PQ::IsEmpty()
{
    return root == NULL;
}

Pop return on failure

Returning -1 on failure means that the tree cannot have nodes with  -1 as a value (a caller cannot tell such node from a failure). Consider throwing an exception instead.
Recursive Push

Push doesn't need to be recursive:

void PQ::Push(int value, Node * node)
{
    while (1) {
        if (value < node->value) {
            if (node->left) {
                node = node->left;
            } else {
                node->left = new Node(value);
                return;
            }
        } else {
            if (node->right) {
                node = node->right;
            } else {
                node->right = new Node(value);
                return;
            }
        }
    }
}

Recursive Pop

Pop needs not to be recursive either:

int PQ::Pop(Node * node, Node * parent)
{
    while (node->right) {
        parent = node;
        node = node->right;
    }
    do_actual_deletion
}

Edit: as requested, no multiple returns, no infinite loops. I am not sure it is cleaner than the original.

    void PQ::Push(int value, Node * node)
    {
        Node ** insertionPoint = 0;
        while (insertionPoint == 0) {
            if (value < node->value) {
                if (node->left) {
                    node = node->left;
                } else {
                    insertionPoint = &node->left;
                }
            } else {
                if (node->right) {
                    node = node->right;
                } else {
                    insertionPoint = &node->right;
                }
            }
        }
        *insertionPoint = new Node(value);
    }


Evaluating Dice Poker results In Dice Poker (or Yacht), a player rolls 5 standard dice (sides 1 - 6) and the resulting roll is given a score based on combinations:

Two matching dice only - pair
Three matching dice only - three
Four matching dice only - four
Five matching dice - yacht
two pairs, full house, small straight, large straight, you get the idea

A player rolls the dice and the input is in a space-delimited format e.g.

3 6 5 6 1 - pair
1 6 6 1 6 - full-house
2 4 3 5 1 - small-straight

I'm reading these results from column A on Sheet1 (we'll say). I'm not convinced I'm checking the hands for results very effectively. It works, but it seems incoherent.

First I have an Enum of the results as well as a function for getting the name of the result -

Option Explicit
Private Enum Combination
    none
    pair
    three
    four
    Yacht
    twopair
    fullhouse
    smallstraight
    bigstraight
End Enum

Private Function GetEnum(ByVal value As Combination) As String
    Select Case value
        Case 0
            GetEnum = "none"
        Case 1
            GetEnum = "pair"
        Case 2
            GetEnum = "three"
        Case 3
            GetEnum = "four"
        Case 4
            GetEnum = "yacht"
        Case 5
            GetEnum = "two-pairs"
        Case 6
            GetEnum = "full-house"
        Case 7
            GetEnum = "small-straight"
        Case 8
            GetEnum = "big-straight"
    End Select
End Function

The meat of it is here:

Sub YachtHands()
    Dim lastRow As Long
    Dim dice As Variant
    lastRow = Sheet1.Cells(Sheet1.Rows.Count, 1).End(xlUp).Row
    Dim index As Long
    Dim result As String
    For index = 1 To lastRow
        dice = Split(Sheet1.Cells(index, 1), " ")
        result = result & EvaluateHand(dice)
    Next
    Debug.Print Trim$(result)
End Sub

Private Function EvaluateHand(ByVal dice As Variant) As String
    Dim score As Combination
    Dim bucket() As Long
    Dim threeFlag As Boolean
    Dim twoFlag As Boolean
    ReDim bucket(1 To 6)
    Dim i As Long
    For i = LBound(dice) To UBound(dice)
        bucket(dice(i)) = bucket(dice(i)) + 1
    Next
    For i = LBound(bucket) To UBound(bucket)
        If bucket(i) > 3 Then
            score = bucket(i) - 1
            GoTo Eval
        End If
        If bucket(i) = 3 Then
            If twoFlag Then
                score = fullhouse
                GoTo Eval
            Else
            threeFlag = True
            End If
        End If
        If bucket(i) = 2 Then
            If threeFlag Then
                score = fullhouse
                GoTo Eval
            ElseIf twoFlag Then
                score = twopair
                GoTo Eval
            Else
                twoFlag = True
            End If
        End If
    Next
    If threeFlag Then
        score = three
        GoTo Eval
    End If
    If twoFlag Then
        score = pair
        GoTo Eval
    End If
    score = CheckStraight(bucket)
Eval:
    EvaluateHand = GetEnum(score) & " "
End Function

Private Function CheckStraight(ByVal bucket As Variant) As Combination
    If bucket(1) = 1 And bucket(2) = 1 And bucket(3) = 1 And bucket(4) = 1 And bucket(5) = 1 Then CheckStraight = smallstraight
    If bucket(2) = 1 And bucket(3) = 1 And bucket(4) = 1 And bucket(5) = 1 And bucket(6) = 1 Then CheckStraight = bigstraight
End Function

I think my straight checking function is sort of ridiculous. My main evaluation also uses a lot of GoTo which in general should be avoided. It also suffers some arrow-code. I think this all stems from whatever algorithm I'm using.

Some general notes:

GetEnum function:

Select Case value
        Case 0
            GetEnum = "none"

This kind of hard-coding of values is very risky, if you change the enumeration your function will break. It should be Case none ...

EvaluateHand

What if you have not exactly 5 numbers?
you're right, usage of GoTo should be very limited, you could avoid majority of it by ElseIf or Select Case
also, I wouldn't have went so much details like the snippet below, just set the flags during the For, then check the flags at the end and set score

 If bucket(i) = 3 Then
            If twoFlag Then
                score = fullhouse
                GoTo Eval

I don't think an array is the best fit for this purpose, I'd suggest to use a dictionary, like here below:

Private Function EvaluateHand_MJ(ByVal dice As Variant) As String
    Dim values As New Dictionary
    Dim i As Long
    Dim result As Combination

    result = Combination.none

    With values
        For i = LBound(dice) To UBound(dice)
            If .Exists(dice(i)) Then
                .Item(dice(i)) = .Item(dice(i)) + 1
            Else
                .Add dice(i), 1
            End If
        Next i

        Select Case .Count  ' check number of distinct values
        Case 1          ' all dices are the same, must be a yacht
            result = Combination.yacht
        Case 2         ' two distinct numbers: four (4+1) or a fullhouse(3+2)
            If .Item(.Keys(1)) = 2 Or .Item(.Keys(1)) = 3 Then
                result = Combination.fullhouse
            Else
                result = Combination.four
            End If
        Case 3         ' 3 distinct: two pairs (2+2+1) or a three (3+1+1)
            For i = 1 To 3
                Select Case .Item(.Keys(i))
                Case Is = 2
                    result = Combination.twopair
                Case Is = 3
                    result = Combination.three
                End Select
            Next i
        Case 4         ' 3 unique numbers and a pair
            result = Combination.pair
        Case 5         ' straight (1 or 6 missing) or nothing
            For i = 1 To 6
                If Not .Exists(CStr(i)) Then
                    If i = 1 Then
                        result = Combination.bigstraight
                    ElseIf i = 6 Then
                        result = Combination.smallstraight
                    End If
                End If
            Next i
        End Select
    End With

    EvaluateHand_MJ = GetEnum(result) & " "
End Function


Coloring weekdays with enums My aim is to assign a color to each workday in a week. So if a user enters a day as a str, then I can know which color it should have.

Because colors and workdays are fixed, I make them as enum.Enum instance.

Is enum.Enum a proper choice for colors and workdays?
To link day and color, I choose dictionary for weekday_color. Is there a better choice?

from enum import Enum

class WeekDay(Enum):
    MONDAY = 'Monday'
    TUESDAY = 'Tuesday'
    WEDNESDAY = 'Wednesday'
    THURSDAY = 'Thursday'
    FRIDAY = 'Friday'

class Color(Enum):
    RED = 'Red'
    GREEN = 'Green'
    BLUE = 'Blue'

weekday_color = {
    WeekDay.MONDAY: Color.RED,
    WeekDay.TUESDAY: Color.GREEN,
    WeekDay.WEDNESDAY: Color.BLUE,
    WeekDay.THURSDAY: Color.GREEN,
    WeekDay.FRIDAY: Color.RED,
}

def main():
    today_input = 'Monday'
    today = WeekDay(today_input)
    today_color = weekday_color[today]
    print(today)
    print(today_color)

if __name__ == '__main__':
    main()

Design-wise, using an Enum is entirely appropriate -- they were added to make "magic" constants easier to work with by giving them informative debugging names and easier containment checks.

Using a dictionary to tie the two enums together is unnecessary in this case as it makes more sense, and is easily possible, to have color be an attribute of each Weekday.

Whether Color is itself an enum depends on what you will be doing with the colors:

do you need actual color values?
is the color value going to be sent to another system, or stay inside your own program?

Whether or not you have a separate Color enum, a good way to combine those two things is like so (shown using aenum1, but can be done with the stdlib Enum2:

from aenum import AutoNumberEnum

class WeekDay(AutoNumberEnum):
    _init_ = 'color'
    #
    MONDAY = 'red'
    TUESDAY = 'green'
    WEDNESDAY = 'blue'
    THURSDAY = 'green'
    FRIDAY = 'red'

and in use:

>>> list(WeekDay)
[<WeekDay.MONDAY: 1>, <WeekDay.TUESDAY: 2>, <WeekDay.WEDNESDAY: 3>, <WeekDay.THURSDAY: 4>, <WeekDay.FRIDAY: 5>]

>>> WeekDay.MONDAY
<WeekDay.MONDAY: 5>

>>> WeekDay['MONDAY']
<WeekDay.MONDAY: 5>

>>> WeekDay.MONDAY.color
'red'

If you decide to use an enum for color, simply assign that instead of, for example, 'red'.

1 Disclosure:  I am the author of the Python stdlib Enum, the enum34 backport, and the Advanced Enumeration (aenum)  library.

2 The stdlib version (without the members):

class WeekDay(Enum):
    #
    def __new__(cls, color):
        value = len(cls.__members__) + 1
        obj = object.__new__(cls)
        obj._value_ = value
        obj.color = color
        return obj
    #
    # members here


this scope for parent object within inner object Is this the best way to get access to the parent scope when dealing with complex objects literals?

I find this a bit ugly, and obviously the best solution would never be involving calling it by name directly, but by saving the state of the previous scope. I tried it with bind() but didn't get it to work properly.

var object = {
　　name : 'foo',
　　getName : (function(){
　　　　　 return function(that){
            return {
                a : 111,
                b : that.name,
                c : function(){
                    return this.b;
                }
            }
       }(this);
 　})
};

object.getName().b  // 'foo'

I understand that you are just experimenting with closures, because your code should be a simple geter like that...

var object = {
　　name : 'foo',
　　getName : function(){ //closure
        return {
            a : 111,
            b : this.name, // variable th is avaliable in the inner scope
            c : function(){
                return this.b;
            }
        }
 　}
};

object.getName().b  // 'foo'

This does the same as your example:

var object = {
　　name : 'foo',
　　getName : (function(){ //closure
         var th=this;     // a new var introduced in a closure
　　　　　 return function(){
            return {
                a : 111,
                b : th.name, // variable th is avaliable in the inner scope
                c : function(){
                    return this.b;
                }
            }
       }();
 　})
};

object.getName().b  // 'foo'

[edit]

this in JS depends on where the function is called as well, so if you want a nice encapsulation Try this:

var object = {}
(function(){

  var _name='foo';

  object.getName=function(){
    return _name;
    }
});


jQuery function that modifies a DIV How can I shorten and optimize this code which displays then hides a DIV?

setTimeout(function () {
    $('.updateNotification').show().addClass('heightener');
    callback();
},2000)

function callback() {
  setTimeout(function() {
    $( '.updateNotification' ).fadeOut();
  }, 10000 );
};

My first thought was to combine the functions. However, I would like them to remain separate because at a later date I will additionally incorporate a button which will trigger callback() in addition to the timer.

I'm not sure how you would shorten this code. However I think that callback is a terrible name for a function that fades a CSS class. Remember that a function signature should represent what a function does and with that said I would rename it to something like fadeUpdateNotification().

And in the fashion of fading you could replace .show() with .fadeIn() to have the div fade in and out - I dunno how your styling/website looks like but that might look nice...


Validating that only certain values are unset I am checking data output in some testing code, and only some fields can be None, but I think this can be better:

for key in data[0].keys():
        self.assertIn(key, data_model)
        if data[0][key] is None:
            self.assertTrue('rate' in key or 'percentage' in key or 'ratio' in key)
            continue
        self.assertIsInstance(data[0][key], data_model[key])

Mostly I don't like the ... or ... in the assertTrue

A few things:

your indentation looks weird
you could store data[0] in some variable with a relevant name.
you could iterate over both keys and values with iteritems() /items()
you do not need continue here. A simple else would do the trick and is more explicit.
The line you don't quite like could be replaced by something like :

self.assertTrue(any(w in key for w in ('rate', 'percentage', 'ratio')))

The resulting code would be :

first_data = data[0]
for key, val in first_data.items():
    self.assertIn(key, data_model)
    if val is None:
        self.assertTrue(any(w in key for w in ('rate', 'percentage', 'ratio')))
    else:
        self.assertIsInstance(val, data_model[key])


Finding all pairs of elements in an array that sum to a target value Is there a shorter way and does not involve nested for loops?

const checkIfSumFromTwoNumbers = (arrayOfNum, targetValue) => {
    let found = [];
    let result = [];

    for (let value of arrayOfNum) {
        if (found[targetValue - value] === true ) {
            result.push({[arrayOfNum.indexOf(value)]: value, [arrayOfNum.indexOf(targetValue-value)]: targetValue - value});
        }
        found[value] = true;
    }
    return result;
};

No, each number requires a second pass to determine whether combined with an existing one it is a match - doesn't mean it can't be short though e.g.

const checkIfSumFromTwoNumbers = (arrayOfNums, targetValue) =>
  arrayOfNums.reduce((matches, val) => {
      const target = arrayOfNums.find(x => (val + x) === targetValue);
      target && matches.push([val, target]);
      return matches;
   }, [])

I was actually ignorant to the fact your code actually works which means it can be done in one pass :) here's a slightly neater / more concise version of what you already have:

const checkIfSumFromTwoNumbers = (arrayOfNum, targetValue) => {
  const seen = {};
  return arrayOfNum.reduce((matches, val) => {
    const delta = targetValue - val;
    seen[delta] && matches.push([delta, val]);
    seen[val] = true;
    return matches;
  }, []);
};


Printing right triangles recursively in Haskell I've implemented a simple program to print out 4 right triangles oriented in different ways to learn Haskell. I'm sure there are more efficient ways of doing what I have done, and I just want some feedback.

All feedback related to the efficiency or quality of my code is welcome.

The four different triangle orientations are listed below

A Top Left Triangle of size 4 would look like:

****
***
**
*

A Bottom Left Triangle of size 4 would look like:

*
**
***
****

A Top Right Triangle of size 4 would look like:

****
 ***
  **
   *

A Bottom Right Triangle of size 4 would look like:

   *
  **
 ***
****

Here is my implemented solution to the problem:

module Main (main) where

import Prelude

-- MARK: Pure section --

makeTopLeftTriangle :: Char -> Int -> [String]
makeTopLeftTriangle c n
    | n < 1     = []
    | otherwise = makeTriangleRow c n : makeTopLeftTriangle c (n-1)
  where
    makeTriangleRow :: Char -> Int -> String
    makeTriangleRow c n
        | n < 1     = []
        | otherwise = c : makeTriangleRow c (n-1)

makeBottomLeftTriangle :: Char -> Int -> [String]
makeBottomLeftTriangle c n = makeBottomLeftTriangleHelper c 1 n
  where
      makeBottomLeftTriangleHelper :: Char -> Int -> Int -> [String]
      makeBottomLeftTriangleHelper c x n
          | x > n     = []
          | otherwise = makeTriangleRow c x : makeBottomLeftTriangleHelper c (x+1) n
        where
            makeTriangleRow :: Char -> Int -> String
            makeTriangleRow c x
                | x <= 0    = []
                | otherwise = c : makeTriangleRow c (x-1)

makeTopRightTriangle :: Char -> Int -> [String]
makeTopRightTriangle c n = [(replicate n c)]
                           ++ zipWith (++) (makeBottomLeftTriangle ' ' (n-1))
                                           (makeTopLeftTriangle c (n-1))

makeBottomRightTriangle :: Char -> Int -> [String]
makeBottomRightTriangle c n = zipWith (++) (makeTopLeftTriangle ' ' (n-1))
                                           (makeBottomLeftTriangle c (n-1))
                              ++ [(replicate n c)]

makeTriangle :: Char -> Int -> Int -> [String]
makeTriangle c triangleType = case triangleType of
    1 -> makeTopLeftTriangle c
    2 -> makeBottomLeftTriangle c
    3 -> makeTopRightTriangle c
    4 -> makeBottomRightTriangle c

-- MARK: Non-pure section --

printTriangle :: [String] -> IO ()
printTriangle []     = return ()
printTriangle (x:xs) = do
    putStrLn x
    printTriangle xs

getTriangleType :: IO String
getTriangleType = do
    putStrLn "What type of triangle do you want to print? (1, 2, 3, or 4)"
    putStrLn "1) Top Left"
    putStrLn "2) Bottom Left"
    putStrLn "3) Top Right"
    putStrLn "4) Bottom Right"
    getLine

main :: IO ()
main = do
    i_triangleType <- getTriangleType
    i_n <- getLine

    let triangleType = read i_triangleType :: Int
    let n = read i_n :: Int

    printTriangle $ makeTriangle '*' triangleType n

The recursion you used is very cumbersome, and should be avoided in favour of more expressive solutions in Haskell.  You figured out how to write replicate n c — why didn't you just run with that?  Add some list comprehensions, and you're done!

Instead of using recursion in printTriangle to print one line at a time, you can just write putStr $ unlines.

Naming each function starting with "make…" feels a bit redundant to me.

Splitting the menu between getTriangleType and makeTriangle is awkward.  You're using an integer to represent the type of triangle, and the two functions have to agree on which number represents which orientation.  The most natural way to represent a type of triangle is to use the corresponding triangle-generating function itself!  And I'd define a type synonym TriangleMaker for those functions.

type TriangleMaker = Char -> Int -> [String]

topLeftTriangle :: TriangleMaker
topLeftTriangle c n = [replicate i c | i <- [n, n-1 .. 1]]

bottomLeftTriangle :: TriangleMaker
bottomLeftTriangle c n = [replicate i c | i <- [1 .. n]]

topRightTriangle :: TriangleMaker
topRightTriangle c n =
  [(replicate (n - i) ' ') ++ (replicate i c) | i <- [n, n-1 .. 1]]

bottomRightTriangle :: TriangleMaker
bottomRightTriangle c n =
  [(replicate (n - i) ' ') ++ (replicate i c) | i <- [1 .. n]]

getTriangleType :: IO TriangleMaker
getTriangleType = do
  let menu = [topLeftTriangle, bottomLeftTriangle, topRightTriangle, bottomRightTriangle]
  putStr $ unlines [
    "What type of triangle do you want to print? (1, 2, 3, or 4)",
    "1) Top Left",
    "2) Bottom Left",
    "3) Top Right",
    "4) Bottom Right"]
  line <- getLine
  return (menu !! ((read line :: Int) - 1))

main :: IO ()
main = do
  triangle <- getTriangleType
  size <- getLine
  putStr $ unlines $ triangle '*' (read size :: Int)


Given an array of integers, return the smallest positive integer not in it Write a function:

function solution(A);

  that, given an array A of N integers, returns the smallest positive
  integer (greater than 0) that does not occur in A.

  For example, given A = [1, 3, 6, 4, 1, 2], the function should return
  5.

  Given A = [1, 2, 3], the function should return 4.

  Given A = [−1, −3], the function should return 1.

  Assume that:

  N is an integer within the range [1..100,000]
  Each element of array A is an integer within the range [−1,000,000..1,000,000]

  Complexity:

  Expected worst-case time complexity is \$O(N)\$
  Expected worst-case space complexity is \$O(N)\$, beyond input storage (not counting the storage required for input arguments)

  Elements of input arrays can be modified.

Solution in \$O(n^2)\$:

function solution(A) {
  for (i = 1; i < 1000000; i++) {
    if(!A.includes(i)) return i;
  }
}

That's a nice simple solution, with two problems:

It will give incorrect result when A contains all the values in the ranges [1..1000000] or [1..999999], returning undefined instead of 1000001 and 1000000, respectively.
It doesn't meet the time complexity requirement, being \$O(n^2)\$ instead of \$O(n)\$.

The first problem is easy to fix by adjusting the end condition of the loop.

The second problem is trickier, and the interesting part of the exercise.
Consider this algorithm, that's \$O(n)\$ in time and \$O(1)\$ in space:

Loop over the elements of A from the start, and for each value A[i], if A[i] - 1 is a valid index in the array, then repeatedly swap A[i] and A[A[i] - 1] until A[i] is in its correct place (value equal to i + 1), or A[i] and A[A[i] - 1] are equal.

This should order the values to their right places such that A[i] == i + 1, when possible

Loop over the elements again to find an index where A[i] != i + 1, if exists then the missing value is i + 1
If the end of the loop is reached without returning a value, then the missing value is A.length + 1.

Here's one way to implement this in JavaScript:

var firstMissingPositive = function(nums) {
    var swap = function(i, j) {
        var tmp = nums[i];
        nums[i] = nums[j];
        nums[j] = tmp;
    };

    for (let i = 0; i < nums.length; i++) {
        while (0 < nums[i] && nums[i] - 1 < nums.length
                && nums[i] != i + 1
                && nums[i] != nums[nums[i] - 1]) {
            swap(i, nums[i] - 1);
        }
    }

    for (let i = 0; i < nums.length; i++) {
        if (nums[i] != i + 1) {
            return i + 1;
        }
    }
    return nums.length + 1;
};

Note: to verify this, or alternative implementations work,
you could submit on leetcode.